9:45 a.m.
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