improvements to contract violation messages updated readme

Author: dkatzel-ncats

README.md

@@ -22,4 +22,16 @@ from your molwitch implementation root directory:
 
 This will run thousands of automated tests that will produce a report when it's
  complete to show which features are Fully Compliant, Partially Compliant or Not Compliant.  
- There may be additional notes or comments about which specific sub-features are not passing.
+ There may be additional notes or comments about which specific sub-features are not passing.
+ 
+ ## How To make Tests Fail
+ By default, the API Checker will still make all the junit tests pass
+ even if they throw exceptions or assertion errors .  This is so a molwitch-implementation
+ that isn't fully compliant can still be move through all the phases of the maven pipeline
+ to be packaged and deployed even if some molwitch features are missing.
+ 
+ To make the failing tests fail the build, set this parameter ot true `molwitch.apiContract.ErrorOnFailure`
+ 
+ ```
+ mvn clean test -Dtest=ApiCheckerSuite -Dmolwitch.apiContract.ErrorOnFailure=true
+``` 

src/test/java/gov/nih/ncats/molwitch/tests/AtomAliasTestApi.java

@@ -22,6 +22,7 @@
 
 import java.util.Optional;
 
+import gov.nih.ncats.molwitch.tests.contract.ApiContract;
 import gov.nih.ncats.molwitch.tests.contract.BasicApiContractChecker;
 import org.junit.Before;
 import org.junit.ClassRule;
@@ -57,32 +58,37 @@ public void setAliasShouldBeEmpty() {
 	}
 
 	@Test
+	@ApiContract(category = "R Group")
 	public void notRGroupWithAliasUnset() {
 		assertFalse(atom.isRGroupAtom());
 		assertTrue(!atom.getRGroupIndex().isPresent());
 	}
 	@Test
+	@ApiContract(category = "R Group")
 	public void notRGroupWithAliasSetToSomethingElse() {
 		atom.setAlias("myAlias");
 		assertFalse(atom.isRGroupAtom());
 		assertTrue(!atom.getRGroupIndex().isPresent());
 	}
 
 	@Test
+	@ApiContract(category = "R Group")
 	public void isRGroupWithAliasSetToRXX() {
 		atom.setAlias("R10");
 		assertTrue(atom.isRGroupAtom());
 		assertEquals(10, atom.getRGroupIndex().getAsInt());
 	}
 
 	@Test
+	@ApiContract(category = "R Group")
 	public void isRGroupWithRGroupSetDirectly() {
 		atom.setRGroup(10);
 		assertTrue(atom.isRGroupAtom());
 		assertEquals(10, atom.getRGroupIndex().getAsInt());
 	}
 
 	@Test
+	@ApiContract(category = "R Group")
 	public void unsetAnAlreadySetRGroup() {
 		atom.setRGroup(10);
 		assertTrue(atom.isRGroupAtom());
@@ -92,6 +98,7 @@ public void unsetAnAlreadySetRGroup() {
 	}
 
 	@Test
+	@ApiContract(category = "R Group")
 	public void unsetAnRGroupWithAnAliasAleadyExisting() {
 		atom.setAlias("foo");
 		atom.setRGroup(null);
@@ -103,6 +110,7 @@ public void unsetAnRGroupWithAnAliasAleadyExisting() {
 	}
 
 	@Test
+	@ApiContract(category = "R Group")
 	public void changeRGroup() {
 		atom.setRGroup(10);
 		atom.setRGroup(5);

src/test/java/gov/nih/ncats/molwitch/tests/CreateChemicalTestApi.java

@@ -38,6 +38,7 @@
 import java.util.stream.Collectors;
 
 import gov.nih.ncats.molwitch.io.*;
+import gov.nih.ncats.molwitch.tests.contract.ApiContract;
 import gov.nih.ncats.molwitch.tests.contract.BasicApiContractChecker;
 import org.junit.Assert;
 import org.junit.ClassRule;
@@ -308,6 +309,7 @@ public void toKekulizedSmilesFile() throws IOException{
 	}
 
 	@Test
+	@ApiContract(category = "Atom Map", message = "Can't Set Atom Map")
 	public void setAtomMap() throws IOException {
 		Chemical chem = Chemical.createFromSmilesAndComputeCoordinates(smiles);
 
@@ -317,13 +319,15 @@ public void setAtomMap() throws IOException {
 		assertEquals(8, chem.getAtom(4).getAtomToAtomMap().getAsInt());
 	}
 	@Test
+	@ApiContract(category = "Atom Map", message = "Can't Clear Atom Map")
 	public void notSetAtomMap() throws IOException {
 		Chemical chem = Chemical.createFromSmilesAndComputeCoordinates(smiles);
 
 		chem.getAtom(4).clearAtomToAtomMap();
 		assertFalse(chem.getAtom(4).getAtomToAtomMap().isPresent());
 	}
 	@Test
+	@ApiContract(category = "Atom Map", message = "Can't Set Atom Map")
 	public void clearAtomMap() throws IOException {
 		Chemical chem = Chemical.createFromSmilesAndComputeCoordinates(smiles);
 

src/test/java/gov/nih/ncats/molwitch/tests/WriteAsMolFileTestApi.java

@@ -21,6 +21,7 @@
 import java.io.IOException;
 
 import gov.nih.ncats.molwitch.io.ChemFormat;
+import gov.nih.ncats.molwitch.tests.contract.ApiContract;
 import gov.nih.ncats.molwitch.tests.contract.BasicApiContractChecker;
 import org.junit.ClassRule;
 import org.junit.Rule;
@@ -127,6 +128,7 @@ public void aromatizeAsISMol() throws IOException{
 	}
 
 	@Test
+	@ApiContract( message = "can't write v3000")
 	public void kekulizeMol3000() throws IOException{
 		Chemical chem = Chemical.createFromSmilesAndComputeCoordinates(smiles);
 		chem.setName("my_id");

src/test/java/gov/nih/ncats/molwitch/tests/contract/ApiContract.java

@@ -27,7 +27,7 @@
 @Retention(RetentionPolicy.RUNTIME)
 public @interface ApiContract {
     public static String DEFAULT_NULL_MESSAGE= "";
-    String category();
+    String category() default DEFAULT_NULL_MESSAGE;
 
     String message()  default DEFAULT_NULL_MESSAGE;
 

src/test/java/gov/nih/ncats/molwitch/tests/contract/ApiContractChecker.java

@@ -53,6 +53,10 @@ protected void before() throws Throwable {
         complianceMessageMap = new HashMap<>();
     }
 
+    protected String getDefaultCategory(){
+        return "";
+    }
+
     @Override
     protected void after() {
         String errorMessage = failIf.apply(map);
@@ -92,6 +96,9 @@ public void evaluate() throws Throwable {
 
                            if(contract!=null){
                                String category = contract.category();
+                               if(category==null ||category.trim().isEmpty() ){
+                                   category=getDefaultCategory();
+                               }
                                String message = contract.message();
                                if(ApiContract.DEFAULT_NULL_MESSAGE.equals(message)){
                                    message =null;

src/test/java/gov/nih/ncats/molwitch/tests/contract/ApiContractSuiteRule.java

@@ -31,6 +31,8 @@ public class ApiContractSuiteRule extends ExternalResource {
     protected void after() {
 //        System.out.println(GlobalApiContractChecker.INSTANCE.getMap());
         System.out.println("====== " + MolWitch.getModuleName() + " COMPLIANCE CONTRACT RESULTS ========");
+
+        System.out.println("| Feature | Compliance Level | Comments|\n | ------ | ---------- | ---------- |");
         try {
             for (Map.Entry<String, Map<ApiContractChecker.ComplianceLevel, SingleThreadCounter>> entry : GlobalApiContractChecker.INSTANCE.getMap().entrySet()) {
                 String category = entry.getKey();
@@ -40,8 +42,8 @@ protected void after() {
                 if (entry.getValue().size() == 1) {
                     Map.Entry<ApiContractChecker.ComplianceLevel, SingleThreadCounter> singleEntry = entry.getValue().entrySet().iterator().next();
                     result = singleEntry.getKey().toString();
-                    System.out.println(category + "\t" + result + ((messageMap==null || messageMap.get(singleEntry.getKey()) == null) ? "" : messageMap.get(singleEntry.getKey()).stream().collect(Collectors.joining("; "))));
-                    System.out.println("--------------------------");
+                    System.out.println("| " + category + "| " + result + " | " + ((messageMap==null || messageMap.get(singleEntry.getKey()) == null) ? "" : messageMap.get(singleEntry.getKey()).stream().collect(Collectors.joining("; "))) + " |");
+
 
                 } else {
                     Comparator<Map.Entry<ApiContractChecker.ComplianceLevel, Long>> SortByLargest = Comparator.<Map.Entry<ApiContractChecker.ComplianceLevel, Long>>comparingLong(e -> e.getValue()).reversed();
@@ -55,9 +57,9 @@ protected void after() {
                                     LinkedHashMap::new));
 
                     for (Map.Entry<ApiContractChecker.ComplianceLevel, Long> entry2 : map.entrySet()) {
-                        System.out.println(category + "\t" + entry2.getKey() + " ( " + entry2.getValue() + " )" + ((messageMap==null || messageMap.get(entry2.getKey()) == null )? "" : messageMap.get(entry2.getKey()).stream().collect(Collectors.joining("; ", " ", ""))));
+                        System.out.println("| " + category + " | " + entry2.getKey() + " ( " + entry2.getValue() + " ) | " + ((messageMap==null || messageMap.get(entry2.getKey()) == null )? "" : messageMap.get(entry2.getKey()).stream().collect(Collectors.joining("; ", " ", ""))) + " |");
                     }
-                    System.out.println("--------------------------");
+//                    System.out.println("--------------------------");
                 }
 
             }

src/test/java/gov/nih/ncats/molwitch/tests/contract/BasicApiContractChecker.java

@@ -35,6 +35,12 @@ public enum TestResult{
     public BasicApiContractChecker(String defaultCategory) {
         this.defaultCategory = defaultCategory;
     }
+
+    @Override
+    protected String getDefaultCategory() {
+        return defaultCategory;
+    }
+
     private Map<TestResult, SingleThreadCounter> testResult = new EnumMap<>(TestResult.class);
 
     @Override