#624 Update to JUnit 4.11 and Hamcrest 1.3

Accepted
closed
nobody
None
master
1
2013-05-07
2013-04-20
No

Discussion

  • John May

    John May - 2013-04-22

    need to update the build.xml also :-)

     
    • Egon Willighagen

      Does it? Mmmm... I thought I had all of that in the .(devel)libdepends now... I will look at it. What is the phenotype of it missing?

       
      • John May

        John May - 2013-04-22

        Ant won't run the tests as the task 'noJunit' looks for 'devellib/junit-4.10'.

         
        Last edit: John May 2013-04-22
        • Egon Willighagen

          Silly me... :(

           
  • John May

    John May - 2013-04-22

    not sure if I could apply this right off so heres the build.xml patch

     
    • Egon Willighagen

      Yeah, best to apply things in one go.

      Signed off.

       
  • John May

    John May - 2013-04-22

    Whoops... just tested full with test-all and the modules also need the hamcrest dependency. This was obviously bundled with the 4.10 version.

     
    • Egon Willighagen

      :)

      Yeah, that needs to be added too, indeed.

       
  • John May

    John May - 2013-05-03

    I'm getting a regression in core from this, but I can't see why... I thought it might have been the other patches as they actually modified CDKAtomTypeMatch but it's definitely these ones which cause it.

    I actually think it might a good fail as the value is lower which implies there are less types which were not tested.

    Testcase: countTestedAtomTypes(org.openscience.cdk.atomtype.CDKAtomTypeMatcherTest): FAILED
    Atom types defined but not tested: Hg.minus Co.plus.4 Co.plus.5 Co.plus.2 Co.plus.6 Mn.3plus Tl O.plus.radical I.sp3d2.3 Mg.neutral.2 Mg.neutral.1 Ge Co.plus Na.neutral Ru.3minus.6 As.3plus Se.sp3.3 Se.sp2.2 I.3 S.thionyl Zn.metallic As Al.3minus Cd.metallic Ti.sp3 Co.plus.1 C.allene Se.sp3.4 Te.4plus Mg.neutral K.neutral N.sp1.2 Se.4plus C.radical.sp1 Hg.plus H.radical Cu.plus N.sp2.radical Rb.plus S.2minus N.sp3.radical H.plus I.minus.5 O.plus.sp1 Ag.neutral Pt.4 C.plus.planar Pt.6 Se.2minus Cl.plus.sp2 expected:<268> but was:<218>
    junit.framework.AssertionFailedError: Atom types defined but not tested: Hg.minus Co.plus.4 Co.plus.5 Co.plus.2 Co.plus.6 Mn.3plus Tl O.plus.radical I.sp3d2.3 Mg.neutral.2 Mg.neutral.1 Ge Co.plus Na.neutral Ru.3minus.6 As.3plus Se.sp3.3 Se.sp2.2 I.3 S.thionyl Zn.metallic As Al.3minus Cd.metallic Ti.sp3 Co.plus.1 C.allene Se.sp3.4 Te.4plus Mg.neutral K.neutral N.sp1.2 Se.4plus C.radical.sp1 Hg.plus H.radical Cu.plus N.sp2.radical Rb.plus S.2minus N.sp3.radical H.plus I.minus.5 O.plus.sp1 Ag.neutral P t.4 C.plus.planar Pt.6 Se.2minus Cl.plus.sp2 expected:<268> but was:<218>
    at org.openscience.cdk.atomtype.AbstractAtomTypeTest.countTestedAtomTypes(AbstractAtomTypeTest.java:245)
    at org.openscience.cdk.atomtype.CDKAtomTypeMatcherTest.countTestedAtomTypes(CDKAtomTypeMatcherTest.java:6615)

     
    • Egon Willighagen

      Confirmed.

      If not mistake this is caused by a bad hack I did earlier. I wanted to make sure to see if all atom types were tested by unit tests, and relied on earlier (undocumented) behavior that the methods were run in order in which they occurred in the .java source file. However, this is no longer the case in the new JUnit, it seems.

      So, I have to fix my tests so that it uses @AfterClass to do this properly, but that is a bit hard, because that requires many more things to be static :/

      I have filed this bug report: https://sourceforge.net/p/cdk/bugs/1299/

      I suggest to go ahead and this patch. I'll fix the unit testing to use @AfterClass as soon as possible.

       
  • John May

    John May - 2013-05-05
    • Group: Needs_Review --> Needs_Revision
     
  • Egon Willighagen

    • Group: Needs_Revision --> Needs_Review
     
  • John May

    John May - 2013-05-07
    • status: open --> closed
    • Group: Needs_Review --> Accepted
     
  • John May

    John May - 2013-05-07

    applied and pushed

     
  • John May

    John May - 2013-05-07

    Oh, it should be noted the changes with atom types convert one fail to an error - but it's the same failure.

     

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks