Fix flaky test FileSearchTests.testBinaryContentTypeWithDescriber #3497
Conversation
|
@HeikoKlare @fedejeanne WDYT? |
|
@HeikoKlare any concerns? |
| String uniqueFolderName= "binaryContentTypeTest-" + System.currentTimeMillis(); | ||
| IFolder folder= ResourceHelper.createFolder(fProject.getFolder(uniqueFolderName)); |
There was a problem hiding this comment.
These tests are JUnit 5, so wouldn't it be better to use the TempDirectory extension to create unique temporary directories? Using currentTimeMillis() to avoid conflicts seems to be prone to errors as well, as no one can guarantee that two tests methods do not call this method at the same time (same time = even sequential but at same system time millis). Note that this private methods is called from two methods which could thus both produce the exact same folder name.
There was a problem hiding this comment.
Just a practical thought: how big are the chances of a name collision happening? We have other tests doing the same trick and they are stable.
There was a problem hiding this comment.
If it has to be under the fProject folder, then maybe just use java.util.UUID.randomUUID().toString()?
There was a problem hiding this comment.
I had many tests failing for such a reason when stabilizing the Platform tests two years ago. Tests might be stable with such a hack, some are like that forever, some are until you move to a different infrastructure (which was the reason for the Platform tests starting to get flaky because they were suddently executed on random machines via GHA instead of dedicated servers running Jenkins).
And in any case, when investing the time into fixing a test anyway, I would be in favor of doing it properly rather than doing something that is prone to fail later on again. But still, better have a fixed test for now than not having any improvement at all, so please take it as a hint/proposal and not as a request from my side.
There was a problem hiding this comment.
Thanks @HeikoKlare @fedejeanne and @sratz Proposal looks great to me, I will ask Claude AI to update the PR with the proposal.
There was a problem hiding this comment.
Isn't it nice that Claude code takes the same amount of time to think about @sratz proposal, agree to it and adjusting the PR, them me writing the above statement?
The test was failing intermittently on Windows with "expected: <1> but was: <2>" because it was searching the entire project scope where files from other concurrent tests could interfere. Changes: - Use unique timestamped folder name to avoid conflicts - Narrow search scope to only the test's specific folder - Add comments explaining the fix This ensures test isolation when running in parallel. Fixes eclipse-platform#3490 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
vogella
force-pushed
the
fix-flaky-file-search-test
branch
from
5172a94 to
8a91157
Compare
|
Thanks everybody for the review and help. |
4 participants
The test was failing intermittently on Windows with "expected: <1> but was: <2>" because it was searching the entire project scope where files from other concurrent tests could interfere.
Changes:
This ensures test isolation when running in parallel.
Fixes #3490
🤖 Generated with Claude Code