Community
Participate
Working Groups
I was debugging the new tests for 324987 (because with Object Teams in the loop two of these tests fail), when I found a dependency that doesn't look kosher to me (the following does not depend on Object Teams!): When running the single test testBug324987_Project02() it fails, when running two tests, testBug324987_Project01() and testBug324987_Project02() all is well. This setup I created with TESTS_NAMES = new String[] { "testBug324987_Project01", "testBug324987_Project02" }; Using this two-tests setup and doing back-to-back debugging I could narrow the issue down to the following difference: If I tweak SetVariableOperation so that it simply returns from its first invocation without doing anything, the second test fails. Note, that I'm effectively crippling the first test, which doesn't mind, but the second test seems to depend on TWO invocations of SetVariableOperation to succeed. The failure I'm seeing is in the assertEquals where I get a "do not insert" although "insert" is expected, i.e., the preferences seem corrupted in some way. The crippling is done in SetVariableOperation with this simple addition: static int count = 0; protected void executeOperation() throws JavaModelException { if ((count++) == 0) return; // continue as normal Can anybody reproduce this behavior? Any idea what might be going on here?
Created attachment 195797 [details] Tweaks exposing the failure This is how far I could get for today: apply this patch and the two-tests setup will fail. This time I only disabled the following and only during the first (unaffected!) test: JavaModelManager.variables.put("JCL_LIB", variablePath); JavaModelManager.variablePreferencesPut("JCL_LIB", variablePath) This crippling produces the exact same effect I see when running only the second test. Re-enabling either statement repairs that second(!) test.
Jay, please investigate. Thanks!
Those two tests and few others have been deprecated, by the way.
(In reply to comment #3) > Those two tests and few others have been deprecated, by the way. Sorry I misunderstood the usage of @deprecated in the tests.
Created attachment 201475 [details] proposed fix Today I saw more non-deterministic behavior of this bug: - PASS with Object Teams in the loop - FAIL with the current patch from bug 349326 After some more debugging I think I found the root cause for non-determinism: org.eclipse.jdt.internal.core.JavaProject.setOptions(Map) loops over the entries of the given Map, so this loop has non-deterministic order. This shouldn't normally hurt, but since bug 324987 JavaModelManager.storePreference(String, String, IEclipsePreferences) has a non-local side effect: if the given option is deprecated also the values for its compatible replacements are modified. Now guess what: if the deprecated option and its compatibles are both contained in the given map, then - if the deprecated option happens to be processed first all is well - if the deprecated option HAPPENS TO BE PROCESSED AFTER its compatible the replacement will implicitly overwrite an explicit value The patch fixes this by passing also the map into storePreferences so that we can avoid this kind of conflicting overwrites. Jay, does this all make sense? I'm currently working on a deterministic test case... if that is possible.
Created attachment 201476 [details] deterministic test Here you go: this test uses a special Map with forced order from entrySet(). Deterministically fails without the patch from comment 5 and passes with that patch :)
I will take a look at the patch and let you know. I have assigned this bug to you, meanwhile.
(In reply to comment #6) > Created attachment 201476 [details] > deterministic test > > Here you go: this test uses a special Map with forced order from entrySet(). > > Deterministically fails without the patch from comment 5 and passes > with that patch :) Stephan, now I understand the fix and the patch looks good. However, this test always passes, with or without the patch. I will try and find out what is wrong.
(In reply to comment #8) > However, this test > always passes, with or without the patch. I will try and find out what is > wrong. I see the option being removed (without the patch) but when it comes back to the test, the option is not null. Is it getting it from somewhere else, perhaps?
Created attachment 202014 [details] Improved and combined patch I don't know what happened to my test, it didn't work because the ForcedOrderMap didn't properly answer to containsKey() requests, thus the testOptions map was seen as being empty (see the for loop in JavaProject.setOptions(Map)). After adding the missing method I can again reproduce the failure, that is now reliably toggled by adding/removing the fix. So, this is what I would release unless you see more issues, Jay?
(In reply to comment #10) > Created attachment 202014 [details] > Improved and combined patch > > I don't know what happened to my test, it didn't work because the > ForcedOrderMap didn't properly answer to containsKey() requests, > thus the testOptions map was seen as being empty > (see the for loop in JavaProject.setOptions(Map)). > > After adding the missing method I can again reproduce the failure, > that is now reliably toggled by adding/removing the fix. > > So, this is what I would release unless you see more issues, Jay? +1 to the patch.
Released in HEAD for 3.8M2
Verified for 3.8M2.
