Travis build is failing

[wowasa]

The Travis build is failing since November, when I added testing for collections. The reason is that all tests of the class CollectionTest are failing when maven is run in batch mode: maven test -B

CollectionTest creates a temporary collection directory and copies a template file slightly modified 100 times into this directory. The test output looks like the collection directory remains empty but no error is thrown.

I could reproduce the test failure in batch mode locally but now even the batch mode is working without failure, although I haven't modified the code

[twagoo]

Wolfgang reports that on his local machines, themvn test -B command succeeds.

I can reproduce it with the same errors with which the Travis builds still fail (example):

[ERROR] Failures:
[ERROR]   CollectionTest.file:162 expected: <99> but was: <35>
[ERROR]   CollectionTest.linkchecking:268 expected: <300> but was: <0>

[twagoo]

The cause of this does not appear to be strictly related to building in batch mode. Locally, I can reproduce the issue both with and without the -b option.

Some underlying issue appears to be present in the code, which may have been obscured by swallowing of an exception in CollectionAggregator.java:89

Also I'm a bit worried about the ugly increment of field variable of a field variable in CollectionAggregator.java:106 although after a quick scan it doesn't seem to do any direct harm.

[twagoo]

Logging the error before shutting down the executor in CollectionAggregator gives me the following information:

2024-05-28T14:49:12.151+02:00 ERROR 23755 --- [ool-5-thread-10] e.c.c.c.a.s.c.CollectionAggregator       : Error occurred while generating report - shutting down executor

java.util.concurrent.ExecutionException: java.lang.NullPointerException: Cannot read field "score" because "instanceReport.facetReport" is null
    at java.base/java.util.concurrent.FutureTask.report(FutureTask.java:122) ~[na:na]
    at java.base/java.util.concurrent.FutureTask.get(FutureTask.java:191) ~[na:na]
    at eu.clarin.cmdi.curation.api.subprocessor.collection.CollectionAggregator$1.afterExecute(CollectionAggregator.java:82) ~[classes/:na]
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145) ~[na:na]
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642) ~[na:na]
    at java.base/java.lang.Thread.run(Thread.java:1583) ~[na:na]
Caused by: java.lang.NullPointerException: Cannot read field "score" because "instanceReport.facetReport" is null
    at eu.clarin.cmdi.curation.api.subprocessor.collection.CollectionAggregator.addReport(CollectionAggregator.java:270) ~[classes/:na]
    at eu.clarin.cmdi.curation.api.subprocessor.collection.CollectionAggregator$2.lambda$visitFile$0(CollectionAggregator.java:133) ~[classes/:na]
    at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:572) ~[na:na]
    at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:317) ~[na:na]
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144) ~[na:na]
    ... 2 common frames omitted

Full log attached below. I think this may be related to #218.

mvn.out.txt

[twagoo]

MANY lines of additional bad direct instance variable access in the same class in the addReport method. Which is also where the above exception originates.

[twagoo]

The above fixes the previously broken CollectionTest.file() test.

CollectionTest.linkchecking() fails but with differing results (i.e. a varying values for collectionReport.linkcheckerReport.totNumOfLinks at CollectionTest.java:268) depending on whether it is run in isolation or together with other tests. Sometimes it even passes.

This to me strongly suggests a concurrency issue. @wowasa I'll leave this for you replicate and investigate.

[wowasa]

the NPE in the CollectionAggregator just had an effect after a recent change for shutdown in case of an exception. Before it was just swallowed by the ThreadPool (and because of this behavior I'm less and less enthusiastic about streams and there functional interfaces). I fixed this new issue and have the old travis issue again, means test are working on my computer with and without B flag while in travis they don't. Last trial now with NotThreadSave annotation, since I can't fix what I can't reproduce locally

[twagoo]

I locally get a test when running mvn clean install on 22bad1272bd880d9833d85c53653a3e0088becbb

[ERROR] eu.clarin.cmdi.curation.api.CollectionTest.linkchecking -- Time elapsed: 0.006 s <<< FAILURE!
org.opentest4j.AssertionFailedError: expected: <300> but was: <0>

[wowasa]

MANY lines of additional bad direct instance variable access in the same class in the addReport method. Which is also where the above exception originates.

actually Davor had a different opinion when he created the project and I, when revised it: Davor made public access to the field variables and logic in the reports. I remained on public access, despite of a view cases where I'm wrapping a non jaxb object, but put the logic into the CollectionAggregator. Since the reports don't do anything else anymore than storing the values, I think it's reasonable to have public field access

[twagoo]

actually Davor had a different opinion when he created the project and I, when revised it: Davor made public access to the field variables and logic in the reports. I remained on public access, despite of a view cases where I'm wrapping a non jaxb object, but put the logic into the CollectionAggregator. Since the reports don't do anything else anymore than storing the values, I think it's reasonable to have public field access

I know there is some history and I can understand why I stayed like that. However nowadays the right choice would probably to use a Record (as in https://openjdk.org/jeps/359) or lombok annotations and where possible try not to sacrifice immutability.