Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C#: Enable Semmle.Util.Tests. #18248

Merged
merged 3 commits into from
Dec 10, 2024
Merged

Conversation

michaelnebel
Copy link
Contributor

@michaelnebel michaelnebel commented Dec 9, 2024

I think there are three issues causing the tests to fail when running them via bazel (one can also be provoked locally).

  • When tests interact with the file system (eg. creating files) they should do so in a special folder: https://bazel.build/reference/test-encyclopedia#test-interaction-filesystem
  • The tests are running in parallel on bazel (this is stated as a comment in our test runner). Since the tests were all interacting with the same file names (copying, creating and deleting) this could cause concurrency issues.
  • When executing File.Delete(path) a directory not found exception can be thrown (this can happen for the "long" path in case the directory has not been created). This can also happen locally, if tests are not executed in the "right order" (som tests explicitly creates the folder containing the "long" path file).

In this PR we use the TEST_TMPDIR environment variable (when set) as the root folder for creating files and folders when running the tests. Furthermore, each test now uses a "unique" (random GUID) for long- and short filenames to avoid collisions when running in parallel and the constructor now initially creates the long path directory before we start running any tests (and some of the tests are adjusted accordingly).

@github-actions github-actions bot added the C# label Dec 9, 2024
@michaelnebel michaelnebel marked this pull request as ready for review December 10, 2024 09:29
@michaelnebel michaelnebel requested a review from a team as a code owner December 10, 2024 09:29
Copy link
Contributor

@tamasvajk tamasvajk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@michaelnebel michaelnebel merged commit 828818d into github:main Dec 10, 2024
17 checks passed
@michaelnebel michaelnebel deleted the csharp/enabletests branch December 10, 2024 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants