-
Notifications
You must be signed in to change notification settings - Fork 162
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Testsuite slim down #1515
Comments
@hmottestad could you take a closer look at the SHACL tests? Executing the build+tests on the SHACL module takes almost 8 minutes (it's responsible for nearly half the build time, on its own). I have a hunch this is related to the recent changes you made to include benchmark tests. I've also noticed that if Jenkins executes two builds in parallel, the JMH benchmarks become flaky: it complains about not being able to create a lock:
I think we should make sure not to run JMH benchmarks as part of the Jenkins build, at least not as part of the regular build-verification jobs. There is an option available to set Jenkins up using fully independent agents, which we could potentially use to schedule regular (nightly?) runs of certain benchmark suites, instead. |
Why does each parallel build server in Jenkins share a temporary directory. Must be some way to enable some decent isolation in Jenkins. |
These are really good tests that have found important bugs in the ShaclSail. |
Some googling didn’t give me much info about what the intention of the jmh lock is, so maybe we should try with -Djmh.ignoreLock=true. |
Thanks for looking into this, I'll first focus on trimming down the other tests. There's big wins there I'm sure. |
Just had a closer look. When you first mentioned this I thought these were shared tests that deal with concurrency and transaction isolation, but they are tests specific to the ShaclSail. Yet they get executed for both native and memory store. If these tests are specifically about how the ShaclSail deals with multithreading, testing against just one backend Sail (either memory or native) should be enough. They both honor the same contracts. Anyway, we can revisit this later. I'll try and focus on some of the shared test cases first, especially the SailConcurrency and RepositoryConnectionTest suite, which I think eat up a lot of time in general. Actually, looking at those more closely, we do a lot of unnecessary duplicate runs there as well, e.g. the RepositoryConnectionTest suite is run against both RDFS inferencers (old and new schema-caching), with both a memory and a native store underneath. I'll try and reorganize this. |
They detected a Concurrent Modification in the TreeModel used by the NativeStore when run with Serializable isolation. This was “fixed” by reordering the calls to .prepare() inside the ShaclSailConnection. I still think it’s actually a bug somewhere in the transaction handling in RDF4J in general, but it’s super hard to figure out since it doesn’t happen when debugging! |
Btw. I’ve previously used maven profiles to run tests based on if the name of the test class ended in something specific. So we could separate out tests that are slow and we don’t need to run that often by renaming to classes to *CI_NIGHTLY_TESTS of something like that. Then have Jenkins fun them periodically or only on commits to develop/master. It should also be possible to do this with java annotations but I’ve never managed to get that to work. Slimming down the maven build plugins might also help. mvn install -dSkipTests has always been fairly slow, even when we had multiple repos. Maybe this is what partially due to the maven plugins? |
That does sound like a bug in the native store. Could we try and reproduce this in a test for the native store directly (that is, without using the SHACLSail on top)? If we know what the SHACLSail tried to do it should be possible to recreate the circumstances. |
I would love that. But I’m barely able to reproduce it through the SHACL sail. And I still don’t know why it’s happening or where the concurrent modification is coming from. |
I made a PR for you to look at @jeenbroekstra wrt. the concurrent modification exception. Btw. My biggest issue right now is the build time. A full clean build took me 11 minutes on my (aging) macbook.
|
Good stuff, thanks @hmottestad . Let's continue discussion on that particular issue there. |
Actually, 11 minutes for a full clean project build doesn't sound too bad (I assume that includes quite a few dependency downloads as well). What exact command did you use to do this? From the times I it looks it doesn't include running the compliance tests, but did this include running unit tests? I'm also noting that creating the rdf4j-client and rdf4j-storage shaded jars together takes up nearly 3 minutes. That might be an easy win: getting rid of those shaded artifacts. |
mvn clean install -DskipTests All dependencies were downloaded already. |
duplicate of CustomGraphcQueryInferencerRepositoryConnectionTest in rdf4j-sail-inferencer, use of native store instead of memory adds little value.
slim down and reorganize test suite
Not quite done, I want to take a closer look at some of the native store tests and the SHACL test suite as well, since those are the two biggest chunks of build time. |
For some reason, after merging this into master, Eclipse IDE is giving me a hard time on project builds. It looks as if its maven project builder can't handle the new structure and gets stuck in a loop. At a loss as to why this didn't happen on the feature branch, tbh. Everything works as expected using command line maven though so for now assuming this is an Eclipse M2E bug. I may need to revisit the idea to use test-jars though. |
A full cleanup of my Eclipse workspace and a re-import of the entire project as a generic Java project (after running Folks: do let me know if your IDE of choice is having trouble with the changed test structure. The point of this exercise is to make it easier, not harder :) |
Unfortunately the issue with Eclipse builds getting stuck in a loop is recurring. Investigating cause and possible fixes (either in our project setup or some config setting in Eclipse I'm overlooking). |
The current tests run (on Jenkins) for close to an hour (54 minutes). Even when disabling compliance tests, building and executing all unit tests still takes over 20 minutes.
We should look into "slimming down" the testuite: identify large/long-running tests, and determine if these tests are still necessary. If not, scrap them. If they are, see if they can be rewritten to run faster.
While we're at it we should also pay attention to the output from tests. Some tests spit out thousands of logger or even STDOUT lines, and it makes trawling through the build log near-impossible.
The text was updated successfully, but these errors were encountered: