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

Fix for testPreTrusted gradle test failing on CI #3479

Merged
merged 4 commits into from
Jan 20, 2022

Conversation

sdedic
Copy link
Member

@sdedic sdedic commented Jan 19, 2022

This PR attempts to fix gradle test suite that is randomly failing just on CI. The root cause is that obviously I've written the test wrong ;) the original idea was to test what will happen if:

  • the project is trusted
  • it has been compiled, closed, IDE restarted
  • the project is opened again.
    Since I didn't want to bring all the NbModuleSuite machinery, the test just moved the original project, created a copy == new FileObject and opened the project again. New FileObject means new Project.

But some Gradle caches rely on j.io.File, not FileObjects, so at this point the prj2 was a new instance, but its GradleFiles key into the cache was the same as the prj key...
... so the project info was found in the cache loaded, and was returned. And the test was failing because sometimes the timestamps of the copied files were different from the timestamps of the in-memory loaded cache data, if CI was lucky to execute the two project loads in different seconds. Then the cache failed, and legacy project loader did not execute the script because it should not do so implicitly on open, but only when (somehow) asked to load the real contents - that is intended and is being tested in Gradle core testsuite.

So I split the test into two cases:

  1. reopening a trusted, closed project w/ caches: this should report proper classpath immediately from the cached data.
  2. reopening a trusted project wil cleared caches: this should initially report some classpath, but as the project is trusted, it should execute the configuration and correct the classpath. This behaviour is desirable when Java files are opened, so that their compilation classpath (and editor underlines) are fixed if the trust allows to execute buildscript.

I need to flush the project info cache from the test, and potentially delete the cache data for project

During the debugging, I've noticed that access to the disk cache was not synchronized and while that did not cause the test failures, it could cause weird things in production (as the hashtable may become corrupted).

I've also encountered a deadlock caused by toString() was trying to synchronize on the project's instance. If the project object was just being formatted by a LogHandler, and some other thread was trying to use Logger in synchronized section using that same project instance, the two threads would block each other.

Then there was a nasty typo that caused the CompletableFuture NOT to complete on success - and therefore Source Groups [were not refreshed[(https://github.com/apache/netbeans/blob/master/java/gradle.java/src/org/netbeans/modules/gradle/java/classpath/ClassPathProviderImpl.java#L159) after project load. This typo was not even noticed by tests ! So I added checks for java2 directory that should appear after full load in ClassPath.SOURCE.

I left some logging tweaks in the test class: it enables FINER logging for the test that was randomly failing, so if this PR does not fix the problem, CI logs will hopefully contain some hints to analyse. I'll remove the setLevel later after the fix proves OK.

@sdedic sdedic self-assigned this Jan 19, 2022
@sdedic sdedic added Gradle [ci] enable "build tools" tests kind:bug Bug report or fix labels Jan 19, 2022
@sdedic
Copy link
Member Author

sdedic commented Jan 19, 2022

Note: this PR need not to be merged into release NB13 (no real functional change), but the failing test is a pain in .... anywhere.

Copy link
Contributor

@lkishalmi lkishalmi left a comment

Choose a reason for hiding this comment

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

Looks good so far.
Could you test if this one also resolves NETBEANS-6117 ?

} catch (IOException ex) {
LOG.log(Level.FINER, "Could not load trust file {0} for projec {1}.", new Object[] { trustFile, project});
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: projec -> project

@sdedic
Copy link
Member Author

sdedic commented Jan 20, 2022

The failed build on Windows is some connectivity issue, cannot be related to the changes (fails at the beginning of build). Merging; will fix the typo in next round of changes.

@sdedic sdedic merged commit 1bdb272 into apache:master Jan 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Gradle [ci] enable "build tools" tests kind:bug Bug report or fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants