[jboss-as7-dev] Testsuite Guidelines

[Jason T. Greene]

Hi Everyone,

Lately I've been seeing a lot of testsuite activity, which is great, 
however I noticed some of the pull requests are doing direct copies of 
the AS6 tests.

One thing to keep in mind is that the AS6 testsuite had a number of 
broken tests, was extremely slow to run, and tested a lot of deprecated 
functionality. Also another issue is that we have quite a few test cases 
in AS7 already, and quite a few of the older tests overlap.

I really would like to keep our testsuite clean and efficient so we do 
not end up with an AS6 style 4 hour testsuite that no one bothers to 
run. This greatly out-ways the need for more coverage (What good is 
extra coverage if its infrequently used?).

Every added test, whether ported or new should follow the same guidelines:

- Verify the test belongs in AS7

AS6 has a lot of tests for things that are discontinued. For example the 
legacy JBoss Transaction Manager which was replaced by Arjuna. Also we 
had tests for SPIs that no longer exist. None of these things should be 
migrated.

- Only add CORRECT and UNDERSTANDABLE tests

If you don't understand what a test is doing (perhaps too complex), or 
it's going about things in a strange way that might not be correct, THEN 
DO NOT PORT IT. Instead we should have a simpler, understandable, and 
correct test. Write a new one, ping the author, or skip it altogether.

- Do not add duplicate tests

Always check that the test you are adding doesn't have coverage 
elsewhere (try using "git grep"). As mentioned above we have some 
overlap between 6 and 7. The 7 test version will likely be better.

- Don't name tests after JIRAs

A JIRA number is useless without an internet connection, and they are 
hard to read. If I get a test failure thats XDGR-843534578 I have to dig 
to figure out the context. It's perfectly fine though to link to a JIRA 
number in the comments of the test. Also the commit log is always available.

- Tests should contain javadoc that explains what is being tested

This is especially critical if the test is non-trivial

- Prefer expanding an EXISTING test over a new test class

If you are looking at migrating or creating a test with similar 
functionality to an exiting test, it is better to
expand upon the existing one by adding more test methods, rather than 
creating a whole new test. In general each
new test class adds at least 300ms to exectution time, so as long as it 
makes sense it is better to add it to an
existing test case.

- Organize tests by subsystem

Integration tests should be packaged in subpackages under the relevant 
subsystem (e.g org.jboss.as.test.integration.ejb.async). When a test 
impacts multiple subsystems this is a bit of a judement call, but in 
general the tests should go into the package of
the spec that defines the functionlaity (e.g. CDI beased constructor 
injection into an EJB, even though this involes CDI and EJB,
the CDI spec defines this behaviour)


- Explain non-obvious spec behavior in comments

The EE spec is full of odd requirements. If the test is covering 
behavior that is not obvious then please add sometihng like "Verifies EE 
X.T.Z - The widget can't have a foobar if it is declared like blah"


- Put integration test resources in the package of the test

At the moment there is not real organization of these files. It makes 
sense for most apps to have this separation, however the testsuite is 
different. e.g. most apps will have a single delpoyment descriptor of a 
given type, for the tesuite will have hundreds, and mantaining mirroring 
package structures is error prone.
This also makes the tests easier to understand, as all the artifacts in 
the deployment are in one place, and that place tends to be small (only 
a handful of files)


-- 
Jason T. Greene
JBoss AS Lead / EAP Platform Architect
JBoss, a division of Red Hat