Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [aspectj-dev] rc2 **Preview Build #4** and new failures (!)

Sorry about wasting George's time by not emailing about those failures, if not fixing the tests.
But my commits didn't break anything.

I did see those two test failures.  The point of fixing the test harness was to show the cases
hidden by the harness adding the output directory to the classpath.  Currently, there is no way
to tell the harness to use the output directory in a full compile from a previous compile, so
the tests were relying on a bug.   I will implement a flag for the <compile> step and
fix this in the tests.

That said, the harness is a diagnostic tool.  I don't think fixing the harness mean you take
responsibility for fixing all the bugs in the test cases or in the components under test.
It's true no commits should be made that cause a regression, but this was instead a bug in
the test case that needed to be seen.  I disagree that adding the output directory back
to the compile classpath was the right fix.  The test author or someone who investigates
could do that, but adding the output directory was a workaround for a bug in incremental
compilation, not for a bug in the test case specification.

However, when I checked in I should have noted those failures so everyone knew that
they needed to be accounted for.  I would amend the rule to say that any new failures in
the tests shown by a commit should be noticed to the mail list.  Before committing, you
must fix any regressions - that is, bugs caused by your commits -  but you need not fix
bugs revealed by your commits.  Showing previously-concealed bugs increases quality.

Rules aside, here rather than just email or notice the messages were the same as for the
inc-compile failure case, I could have seen that they were not inc-compile tests,
and probably would have queried whether they depended on the output directory,
saving George some time.  Certainly good citizens would analyze if not fix the easy
previously-concealed bugs, as I should have.

Wes

Jim.Hugunin@xxxxxxxx wrote:

> George, you made the right quick fix here.  These two (non-incremental) test cases rely on the output directory being on the classpath for the second of their two compiles.  Wes's change last night broke this and he should have caught the problem by running the test suite BEFORE the commit.
>
> Continuing our process of clarifying our developer policies as we go, here's another one that I assumed was obvious:
>
> * cvs commits should only be made after running the test suite -- this includes running the full harness tests in at least one configuration and running the junit tests for any module that you touch
>
> -Jim
>
> > -----Original Message-----
> > From: George Harley1 [mailto:GHARLEY@xxxxxxxxxx]
> > Sent: Friday, May 09, 2003 9:05 AM
> > To: aspectj-dev@xxxxxxxxxxx
> > Subject: [aspectj-dev] rc2 **Preview Build #4** and new failures (!)
> >
> > Hi,
> >
> > Another build of 1.1rc2 has been made and the output aspectj-1.1rc2.jar
> > placed in org.aspectj/releases/aspectj-1.1rc2 on CVS. The test results
> > files have also been updated with the outcome of the unit tests and
> > Harness
> > tests on both 131 and 141 JDKs. An IBM 131 JDK and a Sun 141 JDK were used
> > for the tests.
> >
> > The build was carried out on a 1.3.1 JDK and the CVS extract was done
> > today
> > at about 10:40am BST.
> >
> > JUnit Tests
> > -----------------
> > 100% pass rate running on Sun 1.4.1, Sun 1.3.1 and IBM 1.3.1
> > Results for the Sun 1.4.1 run in junitModules.report.JDK14.zip in the
> > releases directory. Results for IBM 1.3.1 in junitModules.report.zip also
> > in the releases directory.
> >
> > Harness tests
> > ---------------------
> > OK, things start getting a bit interesting here. With the source extract
> > taken for this build all of the harness tests against ajcTests.xml had
> > precisely 2 test failures. The failures were the same across JVMs and were
> > unaffected by the emacssym arg. In both cases a dependency could not be
> > resolved because the output dir of the compile was not on the CLASSPATH.
> > Here is failure #1 ....
> >
> > ------------  FAIL: Reading inner classes from source and bytecode (1) --
> > was failing()
> > --- result: false
> > [  0] ------------  PASS: CompilerRun(CompilerRun.Spec AjdtCommand(2
> > paths))
> > [  0] --- result: true
> > [  0] --- messages
> > [  0] MessageHolder:  (6 info)
> > [  0] [info   0]: info org.aspectj.ajdt.ajc.AjdtCommand@494072f8([-d, C:
> > \Documents and Settings\gharley\Local Settings\Temp\SandboxOHbBVe\classes,
> > -classpath, ..\lib\test\aspectjrt.jar;..\lib\test\testing-client.jar, C:
> > \eclipse\eclipse-2.1-
> > Release\eclipse\workspace\tests\pureJava\innersFromSourceAndBytecode\C.jav
> > a,
> >
> > C:
> > \eclipse\eclipse-2.1-
> > Release\eclipse\workspace\tests\pureJava\innersFromSourceAndBytecode\D.jav
> > a])
> >
> >
> > [  0] [info   1]: info directory classpath entry does not exist: C:
> > \ibm-sdk-131-sr4\jre\classes
> > [  0] [info   2]: info compiling C:
> > \eclipse\eclipse-2.1-
> > Release\eclipse\workspace\tests\pureJava\innersFromSourceAndBytecode\C.jav
> > a
> >
> >
> > [  0] [info   3]: info compiling C:
> > \eclipse\eclipse-2.1-
> > Release\eclipse\workspace\tests\pureJava\innersFromSourceAndBytecode\D.jav
> > a
> >
> >
> > [  0] [info   4]: info weaving
> > [  0] [info   5]: info might need to weave [UnwovenClassFile(C:\Documents
> > and Settings\gharley\Local
> > Settings\Temp\SandboxOHbBVe\classes\C$I$J$K.class, C$I$J$K),
> > UnwovenClassFile(C:\Documents and Settings\gharley\Local
> > Settings\Temp\SandboxOHbBVe\classes\C.class, C), UnwovenClassFile(C:
> > \Documents and Settings\gharley\Local
> > Settings\Temp\SandboxOHbBVe\classes\C$I.class, C$I), UnwovenClassFile(C:
> > \Documents and Settings\gharley\Local
> > Settings\Temp\SandboxOHbBVe\classes\C$I$J.class, C$I$J),
> > UnwovenClassFile(C:\Documents and Settings\gharley\Local
> > Settings\Temp\SandboxOHbBVe\classes\D.class, D)](world=false)
> > [  1] ------------  FAIL: CompilerRun(CompilerRun.Spec AjdtCommand(2
> > paths))
> > [  1] --- result: false
> > [  1] --- messages
> > [  1] MessageHolder:  (7 info)  (2 error)  (2 fail)
> > [  1] [info   0]: info org.aspectj.ajdt.ajc.AjdtCommand@23bc72fa([-d, C:
> > \Documents and Settings\gharley\Local Settings\Temp\SandboxOHbBVe\classes,
> > -classpath, ..\lib\test\aspectjrt.jar;..\lib\test\testing-client.jar, C:
> > \eclipse\eclipse-2.1-
> > Release\eclipse\workspace\tests\pureJava\innersFromSourceAndBytecode\Main.
> > java,
> >
> > C:
> > \eclipse\eclipse-2.1-
> > Release\eclipse\workspace\tests\pureJava\innersFromSourceAndBytecode\C.jav
> > a])
> >
> >
> > [  1] [info   1]: info directory classpath entry does not exist: C:
> > \ibm-sdk-131-sr4\jre\classes
> > [  1] [info   2]: info compiling C:
> > \eclipse\eclipse-2.1-
> > Release\eclipse\workspace\tests\pureJava\innersFromSourceAndBytecode\Main.
> > java
> >
> >
> > [  1] [info   3]: info compiling C:
> > \eclipse\eclipse-2.1-
> > Release\eclipse\workspace\tests\pureJava\innersFromSourceAndBytecode\C.jav
> > a
> >
> >
> > [  1] [info   4]: info CompilerRun.Spec AjdtCommand(2 paths)
> > [  1] [info   5]: info [-d, C:\Documents and Settings\gharley\Local
> > Settings\Temp\SandboxOHbBVe\classes, -classpath, ..
> > \lib\test\aspectjrt.jar;..\lib\test\testing-client.jar, C:
> > \eclipse\eclipse-2.1-
> > Release\eclipse\workspace\tests\pureJava\innersFromSourceAndBytecode\Main.
> > java,
> >
> > C:
> > \eclipse\eclipse-2.1-
> > Release\eclipse\workspace\tests\pureJava\innersFromSourceAndBytecode\C.jav
> > a]
> >
> >
> > [  1] [info   6]: info TestSetup(org.aspectj.ajdt.ajc.AjdtCommand )
> > [  1] [error   0]: error D cannot be resolved or is not a type at C:
> > \eclipse\eclipse-2.1-
> > Release\eclipse\workspace\tests\pureJava\innersFromSourceAndBytecode\Main.
> > java:97:0
> > [  1] [error   1]: error D cannot be resolved or is not a type at C:
> > \eclipse\eclipse-2.1-
> > Release\eclipse\workspace\tests\pureJava\innersFromSourceAndBytecode\Main.
> > java:97:0
> > [  1] [fail   0]: fail unexpected  error: error D cannot be resolved or is
> > not a type at C:
> > \eclipse\eclipse-2.1-
> > Release\eclipse\workspace\tests\pureJava\innersFromSourceAndBytecode\Main.
> > java:97:0
> >
> >
> > [  1] [fail   1]: fail unexpected  error: error D cannot be resolved or is
> > not a type at C:
> > \eclipse\eclipse-2.1-
> > Release\eclipse\workspace\tests\pureJava\innersFromSourceAndBytecode\Main.
> > java:97:0
> >
> >
> > FAIL Reading inner classes from source and bytecode (1) -- was failing()
> >
> > ... and here is failure #2 ....
> >
> > ------------  FAIL: interfaces with non-explicitly static inner classes()
> > --- result: false
> > [  0] ------------  PASS: CompilerRun(CompilerRun.Spec AjdtCommand(1
> > paths))
> > [  0] --- result: true
> > [  0] --- messages
> > [  0] MessageHolder:  (5 info)
> > [  0] [info   0]: info org.aspectj.ajdt.ajc.AjdtCommand@7fb1f2e7([-d, C:
> > \Documents and Settings\gharley\Local Settings\Temp\SandboxZDWzJ2\classes,
> > -classpath, ..\lib\test\aspectjrt.jar;..\lib\test\testing-client.jar, C:
> > \eclipse\eclipse-2.1-
> > Release\eclipse\workspace\tests\pureJava\InterfaceAndInnerHelper.java])
> >
> >
> > [  0] [info   1]: info directory classpath entry does not exist: C:
> > \ibm-sdk-131-sr4\jre\classes
> > [  0] [info   2]: info compiling C:
> > \eclipse\eclipse-2.1-
> > Release\eclipse\workspace\tests\pureJava\InterfaceAndInnerHelper.java
> >
> >
> > [  0] [info   3]: info weaving
> > [  0] [info   4]: info might need to weave [UnwovenClassFile(C:\Documents
> > and Settings\gharley\Local
> > Settings\Temp\SandboxZDWzJ2\classes\InterfaceAndInnerHelper.class,
> > InterfaceAndInnerHelper), UnwovenClassFile(C:\Documents and
> > Settings\gharley\Local
> > Settings\Temp\SandboxZDWzJ2\classes\InterfaceAndInnerHelper$C.class,
> > InterfaceAndInnerHelper$C)](world=false)
> > [  1] ------------  FAIL: CompilerRun(CompilerRun.Spec AjdtCommand(1
> > paths))
> > [  1] --- result: false
> > [  1] --- messages
> > [  1] MessageHolder:  (6 info)  (2 error)  (2 fail)
> > [  1] [info   0]: info org.aspectj.ajdt.ajc.AjdtCommand@5f2532e6([-d, C:
> > \Documents and Settings\gharley\Local Settings\Temp\SandboxZDWzJ2\classes,
> > -classpath, ..\lib\test\aspectjrt.jar;..\lib\test\testing-client.jar, C:
> > \eclipse\eclipse-2.1-
> > Release\eclipse\workspace\tests\pureJava\InterfaceAndInner.java])
> >
> >
> > [  1] [info   1]: info directory classpath entry does not exist: C:
> > \ibm-sdk-131-sr4\jre\classes
> > [  1] [info   2]: info compiling C:
> > \eclipse\eclipse-2.1-
> > Release\eclipse\workspace\tests\pureJava\InterfaceAndInner.java
> >
> >
> > [  1] [info   3]: info CompilerRun.Spec AjdtCommand(1 paths)
> > [  1] [info   4]: info [-d, C:\Documents and Settings\gharley\Local
> > Settings\Temp\SandboxZDWzJ2\classes, -classpath, ..
> > \lib\test\aspectjrt.jar;..\lib\test\testing-client.jar, C:
> > \eclipse\eclipse-2.1-
> > Release\eclipse\workspace\tests\pureJava\InterfaceAndInner.java]
> >
> >
> > [  1] [info   5]: info TestSetup(org.aspectj.ajdt.ajc.AjdtCommand )
> > [  1] [error   0]: error InterfaceAndInnerHelper cannot be resolved or is
> > not a type at C:
> > \eclipse\eclipse-2.1-
> > Release\eclipse\workspace\tests\pureJava\InterfaceAndInner.java:7:0
> > [  1] [error   1]: error InterfaceAndInnerHelper cannot be resolved or is
> > not a type at C:
> > \eclipse\eclipse-2.1-
> > Release\eclipse\workspace\tests\pureJava\InterfaceAndInner.java:8:0
> > [  1] [fail   0]: fail unexpected  error: error InterfaceAndInnerHelper
> > cannot be resolved or is not a type at C:
> > \eclipse\eclipse-2.1-
> > Release\eclipse\workspace\tests\pureJava\InterfaceAndInner.java:7:0
> >
> >
> > [  1] [fail   1]: fail unexpected  error: error InterfaceAndInnerHelper
> > cannot be resolved or is not a type at C:
> > \eclipse\eclipse-2.1-
> > Release\eclipse\workspace\tests\pureJava\InterfaceAndInner.java:8:0
> >
> >
> > FAIL interfaces with non-explicitly static inner classes()
> >
> > It looks like both of the above are related to Wes' changes last
> > night/this
> > morning (see his earlier posting entitled "incremental not resolving
> > unmodified types") which took out the code that added the output directory
> > to the classpath. Taking to heart the advice from the end of his note ("if
> > you are getting problems during incremental
> > compilation with types in unmodified files not being resolved, you can try
> > working around them for now by adding the output directory to the
> > classpath")I made a change to the test harness to add the output dir back
> > into the CLASSPATH and all the tests passed once again. Here is the patch
> > to org.aspectj.testing.harness.bridge.CompilerRun ....
> >
> > ------------------------------8<----------SNIP SNIP-----------8
> > <------------------------
> >
> > Index: CompilerRun.java
> > ===================================================================
> > RCS file:
> > /home/technology/org.aspectj/modules/testing/src/org/aspectj/testing/harne
> > ss/bridge/CompilerRun.java,v
> > retrieving revision 1.7
> > diff -u -r1.7 CompilerRun.java
> > --- CompilerRun.java    9 May 2003 07:14:19 -0000     1.7
> > +++ CompilerRun.java    9 May 2003 15:54:13 -0000
> > @@ -262,9 +262,10 @@
> >
> >          // save classpath and aspectpath in sandbox for this and other
> > clients
> >          final boolean checkReadable = true; // hmm - third validation?
> > -        File[] cp = new File[2 + classFiles.length];
> > +        File[] cp = new File[3 + classFiles.length];
> >          System.arraycopy(classFiles, 0, cp, 0, classFiles.length);
> >          int index = classFiles.length;
> > +        cp[index++] = sandbox.classesDir;
> >          cp[index++] = Globals.F_aspectjrt_jar;
> >          cp[index++] = Globals.F_testingclient_jar;
> >          sandbox.setClasspath(cp, checkReadable, this);
> >
> > ------------------------------>8----------SNIP
> > SNIP----------->8------------------------
> >
> > NOTE : the above has not been committed to CVS.
> >
> > So now the latest 1.1rc2 build passes all of its tests (including
> > installation and post-install tests) but with a tweak to the harness
> > testing that masks the classpath problem highlighted by Wes. What is the
> > consensus on the validity of these tests and hence of this build as our
> > rc2
> > ?
> >
> >
> > Best regards,
> > George
> > ________________________________________
> > George C. Harley
> > IBM e-Business Integration Technologies
> > MP 146, Hursley Park, Winchester, Hants, UK
> > External : +44 (0)1962 819549 / Internal : 7-249549
> > Fax : +44 (0)1962 818999
> > Email : gharley@xxxxxxxxxx
> > Web : http://ibm.com/java
> >
> >
> > _______________________________________________
> > aspectj-dev mailing list
> > aspectj-dev@xxxxxxxxxxx
> > http://dev.eclipse.org/mailman/listinfo/aspectj-dev
> _______________________________________________
> aspectj-dev mailing list
> aspectj-dev@xxxxxxxxxxx
> http://dev.eclipse.org/mailman/listinfo/aspectj-dev



Back to the top