Bugzilla – Full Text Bug Listing
| Summary: | [1.8] Enable reflected parameter names during annotation processing | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Srikanth Sankaran <srikanth_sankaran> | ||||||
| Component: | APT | Assignee: | Jay Arthanareeswaran <jarthana> | ||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||
| Severity: | normal | ||||||||
| Priority: | P1 | CC: | daniel_megert, jarthana, sasikanth.bharadwaj, srikanth_sankaran | ||||||
| Version: | 4.4 | Flags: | jarthana:
review?
(srikanth_sankaran) |
||||||
| Target Milestone: | 4.4 M7 | ||||||||
| Hardware: | PC | ||||||||
| OS: | Windows 7 | ||||||||
| Whiteboard: | |||||||||
| Attachments: |
|
||||||||
|
Description
Srikanth Sankaran
Created attachment 234840 [details] Jesper's patch brought forward from bug 412150 Dani, does this (585 LOC, majority test changes) patch have to go through CQ review ? Jesper has done the committer formalities for JDT/Core, but may already have write access automatically to JDT/APT by virtue of it being in the same repo as JDT/Core. (In reply to comment #2) > Dani, does this (585 LOC, majority test changes) patch have to go through > CQ review ? Jesper has done the committer formalities for JDT/Core, but may > already have write access automatically to JDT/APT by virtue of it being > in the same repo as JDT/Core. APT belongs to the same sub-project (JDT Core). There is no special APT membership/commit right. This seems to have fallen through the cracks - Assigning to Jay so Jesper can focus on debug work This having a patch that I have already reviewed, it will be a shame not to release this for Luna. I will consider this for M7. Looks like the patch still needs couple of changes. 1. The patch introduces 1.8 in the test project's classpath, which we can't guarantee in our test environment. So, that will have to be rolled back. 2. The new tests would fail if run with a 1.7 or below. So, they need to be fixed to be run only with 1.8 and above. I will look at these. Released with changes mentioned above in master: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=b6e21c793f3617f9c60f843e02a0537d5bd09d93 Looks like this causes test failures in our official build (N20140425-2000): Processor reported errors. ----------- Expected ------------ NO ERRORS ------------ but was ------------ Failed during test: Expected fun, but saw arg0. Reason: Wrong name --------- Difference is ---------- expected:<[NO ERRORS]> but was:<[Failed during test: Expected fun, but saw arg0. Reason: Wrong name]> junit.framework.ComparisonFailure: Processor reported errors. ----------- Expected ------------ NO ERRORS ------------ but was ------------ Failed during test: Expected fun, but saw arg0. Reason: Wrong name --------- Difference is ---------- expected:<[NO ERRORS]> but was:<[Failed during test: Expected fun, but saw arg0. Reason: Wrong name]> at org.eclipse.jdt.core.tests.junit.extension.TestCase.assertStringEquals(TestCase.java:250) at org.eclipse.jdt.core.tests.junit.extension.TestCase.assertEquals(TestCase.java:226) at org.eclipse.jdt.apt.pluggable.tests.ModelTests.testMethodParameters(ModelTests.java:139) at org.eclipse.test.EclipseTestRunner.run(EclipseTestRunner.java:657) at org.eclipse.test.EclipseTestRunner.run(EclipseTestRunner.java:310) at org.eclipse.test.CoreTestApplication.runTests(CoreTestApplication.java:36) at org.eclipse.test.CoreTestApplication.run(CoreTestApplication.java:32) at org.eclipse.equinox.internal.app.EclipseAppContainer.callMethodWithException(EclipseAppContainer.java:587) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:198) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:134) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:104) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:379) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:233) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:648) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:603) at org.eclipse.equinox.launcher.Main.run(Main.java:1465) at org.eclipse.equinox.launcher.Main.main(Main.java:1438) at org.eclipse.core.launcher.Main.main(Main.java:34) The test result is inconsistent. While the tests failed both in Linux and Window in the previous build, it passed in Windows in the last build. The only cause I see MethodBinding.parameterNames is not properly set. I don't know if preserving (or not) the local variable names has something to do with it. The testcase explicitly enables the option to preserve the parameter names by the compiler. It will be interesting to see what the compiled class files look like. I will request releng to help me get those. (In reply to Jayaprakash Arthanareeswaran from comment #9) > The testcase explicitly enables the option to preserve the parameter names > by the compiler. It will be interesting to see what the compiled class files > look like. I will request releng to help me get those. You don't need releng for that, just download the tests (141 MB): http://download.eclipse.org/eclipse/downloads/drops4/N20140426-1500/download.php?dropFile=eclipse-Automated-Tests-N20140426-1500.zip (In reply to Dani Megert from comment #10) > You don't need releng for that, just download the tests (141 MB): > http://download.eclipse.org/eclipse/downloads/drops4/N20140426-1500/download. > php?dropFile=eclipse-Automated-Tests-N20140426-1500.zip This is not what we want. We want to look at some of the classes generated by the failing tests. I don't think this will have those. In fact, thinking about it, the generated ones most likely won't still be available as subsequent tests could have overwritten them :( (In reply to Jayaprakash Arthanareeswaran from comment #11) > This is not what we want. We want to look at some of the classes generated > by the failing tests. I don't think this will have those. In fact, thinking > about it, the generated ones most likely won't still be available as > subsequent tests could have overwritten them :( I see. You could use the disassembler to verify that the class file is OK (or not). The failure symptoms indicate that the OPTION_MethodParametersAttribute compiler preference didn't seem to have any effect. I can simulate this error by commenting out that line of code: jproj.setOption(CompilerOptions.OPTION_MethodParametersAttribute, CompilerOptions.GENERATE); The IJavaProject#setOption() has been a problem area before. If I don't get any breakthrough, will put some hypothetical fix with some trouble shooting logs for the next build. Created attachment 242394 [details] Fix for test failure Looks like some of the changes made in bug 405025 in ClassFile() had to be put in ClassFile#reset() also. This was resulting in unpredictable behavior in some cases. This patch addresses this. Will release after running all tests. Srikanth, can you please review the patch. TIA Released in master for the next I build. http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=f74cac509deaf512c7535f38357f0bf074e0e26d Verified for 4.4 M7 using I20140429-2000 build |