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.

Sign up for GitHub

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

Jump to bottom

CacheResourcesManagerTest.testAvailableCache fails sometimes #753

Closed
angelozerr opened this issue May 31, 2020 · 3 comments · Fixed by #883
Closed

CacheResourcesManagerTest.testAvailableCache fails sometimes #753

angelozerr opened this issue May 31, 2020 · 3 comments · Fixed by #883
Assignees
@datho7561
Labels
bug Something isn't working debt This issue or enhancement is related to technical debt
Milestone
0.14.0

Comments

@angelozerr
Copy link
Contributor

The Junit test CacheResourcesManagerTest.testAvailableCache fails sometimes in CI build. I suspect that several PR uses the same folder to check file is not downloaded. I think teh fix is to use a temporar file to download XSD, DTD files to test.
@angelozerr angelozerr mentioned this issue May 31, 2020
Merged
@angelozerr angelozerr added the bug Something isn't working label Jun 10, 2020
@datho7561
Copy link
Contributor

I'd be interested in investigating this. It happens on my local machine as well.

@datho7561 datho7561 self-assigned this Sep 15, 2020
@fbricon fbricon added the debt This issue or enhancement is related to technical debt label Sep 17, 2020
@datho7561
Copy link
Contributor

I was having some issues reproducing it on my local machine. On a Fedora VM where I limit the bandwidth access, CacheResourcesManagerTest.testUnavailableCache fails. If I increase the cache expire time (just for the tests) to 2 seconds and wait 1 second for files to download, then it works. This will increase the test run time by a bit, but is a fiarly simple fix.

Another way to go about this (cleaner implementation, which requires a lot more changes and is probably not feasible) is to return a CompletableFuture<Path> in the cache's getResource, and refactor all the code to use futures.

datho7561 added a commit to datho7561/lemminx that referenced this issue Sep 22, 2020
@datho7561
Cache test uses polling to adapt to system speed
e7597f3 
Instead of waiting a fixed amount of time for a download,
the cache tests use repeated polling to check if the download has finished.
This means that on fast systems, the tests will run faster,
and on slow systems there should be less random failure.

Fixes eclipse-lemminx#753

Signed-off-by: David Thompson <[email protected]>
@datho7561 datho7561 mentioned this issue Sep 22, 2020
Merged
@datho7561
Copy link
Contributor

I tried again with cpulimit to limit the CPU usage to 25%, and both the available and unavailable tests failed consistently.

I also tried with trickle to limit download bandwidth to 1kbps. This also caused tests to fail, but not as consistently as when limiting the CPU.

datho7561 added a commit to datho7561/lemminx that referenced this issue Sep 22, 2020
@datho7561
Cache test uses polling to adapt to system speed
8ccaf15 
Instead of waiting a fixed amount of time for a download,
the cache tests use repeated polling to check if the download has finished.
This means that on fast systems, the tests will run faster,
and on slow systems there should be less random failure.

Fixes eclipse-lemminx#753

Signed-off-by: David Thompson <[email protected]>
datho7561 added a commit to datho7561/lemminx that referenced this issue Sep 22, 2020
@datho7561
Cache test uses polling to adapt to system speed
3eee5c4 
Instead of waiting a fixed amount of time for a download,
the cache tests use repeated polling to check if the download has finished.
This means that on fast systems, the tests will run faster,
and on slow systems there should be less random failure.

Fixes eclipse-lemminx#753

Signed-off-by: David Thompson <[email protected]>
datho7561 added a commit to datho7561/lemminx that referenced this issue Sep 22, 2020
@datho7561
Cache test uses polling to adapt to system speed
4b65fc8 
Instead of waiting a fixed amount of time for a download,
the cache tests use repeated polling to check if the download has finished.
This means that on fast systems, the tests will run faster,
and on slow systems there should be less random failure.

Fixes eclipse-lemminx#753

Signed-off-by: David Thompson <[email protected]>
datho7561 added a commit to datho7561/lemminx that referenced this issue Sep 22, 2020
@datho7561
Cache test uses polling to adapt to system speed
7b37702 
Instead of waiting a fixed amount of time for a download,
the cache tests use repeated polling to check if the download has finished.
This means that on fast systems, the tests will run faster,
and on slow systems there should be less random failure.

Fixes eclipse-lemminx#753

Signed-off-by: David Thompson <[email protected]>
datho7561 added a commit to datho7561/lemminx that referenced this issue Sep 22, 2020
@datho7561
Cache test uses polling to adapt to system speed
d57dc42 
Instead of waiting a fixed amount of time for a download,
the cache tests use repeated polling to check if the download has finished.
This means that on fast systems, the tests will run faster,
and on slow systems there should be less random failure.

Fixes eclipse-lemminx#753

Signed-off-by: David Thompson <[email protected]>
datho7561 added a commit to datho7561/lemminx that referenced this issue Oct 2, 2020
@datho7561
Use CompletableFuture in CacheResourceManagerTest
b216ac9 
Use a CompletableFuture to wait for the download to finish
when testing the CacheResourceManager.
This should mean different CPU/network conditions don't
cause the tests to fail.
A generous timeout is given for the downloads.

Fixes eclipse-lemminx#753

Signed-off-by: David Thompson <[email protected]>
datho7561 added a commit to datho7561/lemminx that referenced this issue Oct 2, 2020
@datho7561
Use CompletableFuture in CacheResourceManagerTest
3e2118b 
Use a CompletableFuture to wait for the download to finish
when testing the CacheResourceManager.
This should mean different CPU/network conditions don't
cause the tests to fail.
A generous timeout is given for the downloads.

Fixes eclipse-lemminx#753

Signed-off-by: David Thompson <[email protected]>
@angelozerr angelozerr added this to the 0.14.0 milestone Oct 13, 2020
angelozerr pushed a commit that referenced this issue Oct 13, 2020
@datho7561 @angelozerr
Use CompletableFuture in CacheResourceManagerTest
f160854 
Use a CompletableFuture to wait for the download to finish
when testing the CacheResourceManager.
This should mean different CPU/network conditions don't
cause the tests to fail.
A generous timeout is given for the downloads.

Fixes #753

Signed-off-by: David Thompson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@datho7561 datho7561

Labels
bug Something isn't working debt This issue or enhancement is related to technical debt
Projects
None yet
Milestone
0.14.0
Development

Successfully merging a pull request may close this issue.

Use CompletableFuture in CacheResourceManagerTest datho7561/lemminx
3 participants
@fbricon @angelozerr @datho7561