Cleanup test suite

26 views
Skip to first unread message

Jan.J...@gdata.de

unread,
Oct 9, 2018, 8:11:04 AM10/9/18
to janusgr...@googlegroups.com

Hi all,


During my work on Testcontainers (https://github.com/GDATASoftwareAG/janusgraph/tree/testcontainers), I needed to modified all tests files multiple times. These multiple changes can follows to mistakes in implementations. Besides, if something in the configuration has changed, you probably need to update all test classes.


Janusgraph test classes should become better manageable, for example, add new test classes, split up tests or detect difference between backend (completeness). 

I found two possible solution to improve:


1. Combine all tests into test suite, see PR https://github.com/JanusGraph/janusgraph/pull/1259 . My proof of concept based on solution from Tinkerpop with some changes for Janusgraph.

+ Reduced boilerplate, one provider file for one database backend and a suite file to define tests.

+ Improved ignoring of tests, currently tests are overwritten with an empty method. These method doesn't prevent to execute the startup and cleanup method for these ignored tests. (Performance improvement)

+ Changes of database configuration need to be done only once.

- Tests are only executable behind a test suite. (Can be probably fixed)

- It could only exists one provider per test project. (This need to be fixed)


2. Using Junit Rule for configuration.

+ Reduced boilerplate, one provider file for one database backend. 

+ Less code change (Smaller review).

+ Changes of database configuration need to be done only once.

-  All test classes need to be exists for all backends. (Current state) (An idea to write all of theses classes would be to auto generate these test classes.)


Besides, an upgrade to Junit 5 from Junit 4 can be a benefit in both cases, see https://junit.org/junit5/docs/current/user-guide/ .

I would appreciate any feedback to both present solution or do you have another possible solution? What should be the next steps to improve the current situation?

 

Greetings,

Jan

Florian Hockmann

unread,
Oct 16, 2018, 6:54:23 AM10/16/18
to JanusGraph developers
I talked briefly with Jan about this after he created his WIP PR and recommended to ask here on the list before refactoring basically the whole test suite of JanusGraph for recommendations on how to proceed.

So, if anyone has a strong opinion on any of the two options Jan presented or maybe even against making such big changes to the test suite in general, then please come forward and express your concerns / your preferences.
Otherwise, I guess he can only decide on his own which option makes most sense for JanusGraph and then create a PR with that changes (ideally maybe a minimal first PR with changes for just 1-2 backends or test classes so we have something so to review).

Jason Plurad

unread,
Oct 18, 2018, 2:19:15 PM10/18/18
to JanusGraph developers
Jan, thanks for digging into this. In the #2 option, you mentioned that "All test classes need to be exists for all backends. An idea to write all of theses classes would be to auto generate these test classes." Would the generated test classes be done dynamically at build time or would it be done once and checked into source control? If it is the latter, my concern would be that the test classes would get out of sync across different backends.

Jan.J...@gdata.de

unread,
Oct 19, 2018, 2:23:37 AM10/19/18
to janusgr...@googlegroups.com

Hi Jason,
I known from a different project which generated classes. These are generated during a extra build task. This allows to have the test class source, but prevent such problems as different backends would get out of sync. We only need to make sure these generated test classes are not cached during each travis build.


I see this more as a second step to generate class.


Dear Jan


Von: janusgr...@googlegroups.com <janusgr...@googlegroups.com> im Auftrag von Jason Plurad <plu...@gmail.com>
Gesendet: Donnerstag, 18. Oktober 2018 20:19:14
An: JanusGraph developers
Betreff: Re: Cleanup test suite
 
--
You received this message because you are subscribed to the Google Groups "JanusGraph developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to janusgraph-de...@googlegroups.com.
To post to this group, send email to janusgr...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/janusgraph-dev/ef1f3fdd-d0d9-4cb9-925d-aac5624d434f%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply all
Reply to author
Forward
0 new messages