Community
Participate
Working Groups
When verifying bug 357425 I failed to reproduce the reported regression. - interactively performing the operations outlined in bug 357425 comment 0 using 3.8 M2 I could not see any misbehavior. Not sure if I misunderstood the problem description of if the bug can only be triggered programmatically - from the two tests in bug 357425 only one (testBug357425()) shows a difference with/out the patch, while testTransitionFromInvalidToValidJar() is green regardless. Since the bug reports about two error scenarios it might be good to ensure that both are indeed captured by a regression test.
(In reply to comment #0) > When verifying bug 357425 I failed to reproduce the reported regression. > > - interactively performing the operations outlined in bug 357425 comment 0 > using 3.8 M2 I could not see any misbehavior. Not sure if I misunderstood > the problem description of if the bug can only be triggered programmatically It is only triggered under the following circumstances: * the libraries on the classpath are updated, and when the DeltaProcessor code runs one of the library jars does not yet exist as a file * the missing library jar gets created * the DeltaProcessor code gets invoked a second time This is a fairly common experience at Google, because we put a lot of dynamically generated jar files on the Eclipse classpath, but would be difficult to interactively recreate in a different environment that uses static classpath jars. > > - from the two tests in bug 357425 only one (testBug357425()) shows a > difference with/out the patch, while testTransitionFromInvalidToValidJar() > is green regardless. I don't understand that, the test fails consistently for me without the patch since validateClasspathEntry() never clears out a cached invalid archive entry (I did TDD so the test failed before it passed.) > > Since the bug reports about two error scenarios it might be good to ensure > that both are indeed captured by a regression test. Please recheck testTransitionFromInvalidToValidJar(), I think you will find it does fail without the patch.
(In reply to comment #1) > [...] > This is a fairly common experience at Google, because we put a lot of > dynamically generated jar files on the Eclipse classpath, but would be > difficult to interactively recreate in a different environment that uses static > classpath jars. I see, so let's not worry about the interactive case. I just tried to reproduce somehow. > > - from the two tests in bug 357425 only one (testBug357425()) shows a > > difference with/out the patch, while testTransitionFromInvalidToValidJar() > > is green regardless. > > I don't understand that, the test fails consistently for me without the patch > since validateClasspathEntry() never clears out a cached invalid archive entry > (I did TDD so the test failed before it passed.) Hm, the most obvious difference is: I started from HEAD and then reverse-applied your patch - except for the tests. I will recheck and report back here.
(In reply to comment #2) > > I don't understand that, the test fails consistently for me without the patch > > since validateClasspathEntry() never clears out a cached invalid archive entry > > (I did TDD so the test failed before it passed.) > > Hm, the most obvious difference is: I started from HEAD and then > reverse-applied > your patch - except for the tests. I will recheck and report back here. I hate to say, but I couldn't find a version of the JDT/Core where the test testTransitionFromInvalidToValidJar() would fail. I tried the version right before the patch for bug 357425 was committed and I also tried a plain 3.8M2. Adding just your tests and running them always gives: - testTransitionFromInvalidToValidJar PASS - testBug357425 FAIL The same results even with the older tests from bug 357425 comment 10. I'm not saying we have a bug here, I'd just like to better understand what's going on here. Maybe Jay has an idea?
Created attachment 205953 [details] Update the ValidateClasspathEntry test to set up an invalid jars rather than non-existing jars My apologies to Stephan. I did the majority of my testing against 3_7_maintenance. In 3.7, non-existing jars are added to the invalid archives list, whereas in 3.8 they are not added to that list. Changing the test setup to create empty files rather than non-existent files displays the behavior we are trying to fix in ClasspathEntry.validateClasspathEntry(), which is that once a jar file is added to the invalid archives list, validateClasspathEntry() will always report the file as invalid, even if the file gets updated. That hopefully solves the mystery.
Created attachment 206201 [details] patch for master Terry, your patch does not apply on master. I'm attaching the correct patch. Please confirm if its ok. I tested by reverting the fix and confirmed that the tests pass only with the fix.
(In reply to comment #5) > Created attachment 206201 [details] > patch for master > > Terry, your patch does not apply on master. I'm attaching the correct patch. > Please confirm if its ok. I tested by reverting the fix and confirmed that the > tests pass only with the fix. Yes, I generated that patch based on the file state before the tests were added. Sorry about that. I had to strip the "a" and "b" path prefixes from your patch to get it to apply (from inside Eclipse rather than the command line). Once it applied the patch looked good, and I saw the test fail when the change to ClasspathEntry.validateClasspathEntry() was reverted. So the patch looks good.
Released in master for 3.8M4 via commit 563f107c1a60920f25930d8da70dd8b0be61372c. Thanks Stephan and Terry!
Verified for 3.8M4 by code inspection.
.
