[Linux] Fix random test failures of browser tests #1523 #1788
Conversation
HeikoKlare
force-pushed
the
file-descriptors-browser-tests
branch
from
5a43b26 to
4a338c6
Compare
|
Further remark: I have integrated that kind of change in #1706 for several builds of that PR and the tests always succeeded on Jenkins. |
| // by other Maven plugins executed in parallel build (such as parallel | ||
| // compilation of the swt.tools bundle etc.) | ||
| String resolvedPath = Files.isSymbolicLink(f) ? Files.readSymbolicLink(f).toString() : f.toString(); | ||
| if (!resolvedPath.contains(".m2")) { |
There was a problem hiding this comment.
should we extract this to a method (isValidPath) maybe? Then it could be enhanced later on.
We most likely want to exclude java files or jar in general, so thinking further maybe even better have a white list to what kind of descriptors do we expects a browser can leave open at all?
There was a problem hiding this comment.
Yes, that makes sense. I have extended the PR with a minor refactoring of the file descriptor processing and extracted the filtering into a separate method.
Regarding an extensive whitelist: generally, having such a list with kinds of descriptors we should not consider sounds reasonable to me. However, I am not sure how that list has to look like to not exclude files that would actually be of interest. Since the current implementation of the file descriptor analysis is rather weak anyway, I am not even sure how much sense it makes to target a "precise" whitelist. The analysis currently contains workarounds for surefire leaving behing open file descriptors (of which I am not sure whether that is still true) and having some random limit of open file descriptors (50) anway. See also #108 for this.
So I would summarize as follows:
- The analysis is capable of catching high numbers of file descriptors being left open after a browser test was executed
- The analysis is imprecise, possibly leading to false positives
- We want to keeo the number of false positives as low as possible by reducing the number of unrelated file descriptors being taken into account
HeikoKlare
force-pushed
the
file-descriptors-browser-tests
branch
6 times, most recently
from
47f8e1d to
f7e9819
Compare
|
Just saw this in a recent build, so might be that this change will not catch all "open file descriptors" failures as there may be file descriptors of which we cannot decide if they are related to the test or not. But this change will hopefully at least significantly reduce the number of false "positives": |
Browser tests randomly fail during Jenkins execution because of too many opened file descriptors. The reason seems to be that the build uses parallel execution and Maven plugins executed for other bundles may have opened and closed file descriptors in parallel, which are erroneously taken into account by the browser tests evaluating the number of file descriptors left open after a test execution. This change excludes open file descriptors for Maven artifacts used in parallel by other Maven plugins by not considering file descriptors with their path containing ".m2" or "target/classes". Fixes eclipse-platform#1523
HeikoKlare
force-pushed
the
file-descriptors-browser-tests
branch
from
f7e9819 to
f19242f
Compare
It looks like it does it quite successfully. I saw many more builds pass today. Thanks a lot! |
Browser tests randomly fail during Jenkins execution because of too many opened file descriptors. The reason seems to be that the build uses parallel execution and Maven plugins executed for other bundles may have opened and closed file descriptors in parallel, which are erroneously taken into account by the browser tests evaluating the number of file descriptors left open after a test execution.
This change excludes open file descriptors for Maven artifacts used in parallel by other Maven plugins by not considering file descriptors with their path containing ".m2" or "target/classes".
Fixes #1523
Example
In previous logs, you find lists of analyzed file descriptor deltas like this:
Remark
The currently used validation approach of browser tests on Linux relies on some global system state (open file descriptors at system level). This makes the approach error-prone in general. Actually, it would require far more isolation of test execution than currently achieved. A further improvement would be to ensure that, at least during the Maven build, no other task is executed in parallel (such as the compilation of another bundle), as that is the reason for the open descriptors of files in
.m2directory but that can of course also open other file descriptors, such as intargetfolders of the bundles. That would not prevent completely independent tasks opening file descriptors concurrently, but since the analysis is very weak anyway (it allows a delta of 50 + test number more open file descriptors between the state before test execution and after a single test). That's also the reasons why it seems to be sufficient to exclude.m2, as that involves a high number of files (more than the 50 + test number allowed ones).I am not even sure if the overall analysis is that useful in it's current implementation (in particular as it is so weak, basically allowing every test to leave behind another open file descriptor). So maybe it would even be necessary to revise or remove that analysis.
At least, this should point us to the cause of the frequent Jenkins build failures.