| Summary: | Optimize unit tests w.r.t. store setup/teardown | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Modeling] EMF | Reporter: | Caspar D. <caspar_d> | ||||||||||||
| Component: | cdo.releng | Assignee: | Caspar D. <caspar_d> | ||||||||||||
| Status: | CLOSED FIXED | QA Contact: | Eike Stepper <stepper> | ||||||||||||
| Severity: | enhancement | ||||||||||||||
| Priority: | P3 | CC: | saulius.tvarijonas, stefan, stepper | ||||||||||||
| Version: | 4.0 | Flags: | stepper:
review+
caspar_d: review? (stepper) |
||||||||||||
| Target Milestone: | --- | ||||||||||||||
| Hardware: | All | ||||||||||||||
| OS: | All | ||||||||||||||
| Whiteboard: | |||||||||||||||
| Bug Depends on: | 337180 | ||||||||||||||
| Bug Blocks: | |||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Caspar D.
(In reply to comment #0) > Especially when using a DB store, this makes execution of > the entire test suite pretty slow. Personally, I find this > a serious nuisance. Agreed. > I think if we (me?) spend a bit of time on this, there > should be ways of drastically reducing the setup/teardown > overhead. That wold be great! I've assigned this bug to you ;-) > - Having tests declare whether they need a clean store or not, > e.g. boolean AbstractCDOTest.needsCleanStore(), or more > finegrained control, like mustDropData(), mustDropPackages(). What about method-level annotations? > - Generating unique resource names in tests, instead of using > hardcoded "/res1" in pretty much every test. This would > isolate the test resource contents. I'm not sure if/how that could work without modifying each test case. If we have to touch each of them I'd prefer to either adjust the resource paths by hand or use the above drop mechanism. In any case we seem to introduce some potential for unpredictability. How can a test case decide on its own whether it's safe reuse *some* former state or not? I think we can never be sure that a given test does not depend on *any* previous state. For one given execution order we could probably say whether a test depends on this particular previous test or not, but if the order changes you know nothing again. What about this, we start each test case without cleaning the previous state. If it fails, we clean the previous state, execute it again and only if it fails again, we propagate the failure? If we log the retries separately we may get a picture of 1) which tests should be executed "clean" without wasting time for a dirty try. Annotations could be added to these tests to force a clean state 2) whether it's worth it at all ;-) (In reply to comment #2) > Annotations could be added to these tests to force a clean state I'm moving ahead with the method annotations. Looks good so far, will probably have a patch annotating all tests that need it, by tomorrow. Did someone set Component=cdo.releng intentionally here? Or did I accidentally set it that way when I opened it? Makes no sense to me. Created attachment 189167 [details]
Patch v1
Created attachment 189786 [details]
Patch v2
BTW. I found that my anti virus software consumes considerable resources, too, because it watches the system temp folder. I've added the ability to override the temp folder with a file named "org.eclipse.net4j.util.io.tmpdir" in the user.home folder. It may specify "path=/alternative/temp/folder" to place the temp folder somewhere outside the virus scanner's scope. In addition I turned off tracing in the H2 suites. I have the feeling the results are already better. In seconds: 419 branching 482 branching, range-based 542 branching, range-based, copyOnBranch 688 audit, range-based 694 audit 612 non-audit Created attachment 189916 [details]
Patch v3
I'm not the expert with the DStore tests but it seems that there's more than just drop logic in AllTestsDBHsqldb.Hsqldb.deactivateRepositories():
if (USE_VERIFIER)
{
IRepository testRepository = getRepository(REPOSITORY_NAME);
if (testRepository != null)
{
getVerifier(testRepository).verify();
}
}
I suspect that should be executed on each tearDown...
(In reply to comment #7) > BTW. I found that my anti virus software consumes considerable resources, too, > because it watches the system temp folder. That's a good tip for Windows users, but I'm not one of them. My platform doesn't require one, hehehe. > In addition I turned off tracing in the H2 suites. I have tracing turned off always. See my branches/caspar/options branch in our SVN, to which I keep my .options files switched. Point is: the improvements I mentioned on Skype the other day, were already free of the influences of virus scanners & tracing. (In reply to comment #9) > That's a good tip for Windows users, but I'm not one of them. My platform > doesn't require one, hehehe. Yeah, I heard of some operating systems that are unattractive enough :P Please fix the Verifier issue and commit. Committed to trunk, rev. 7305 Created attachment 193559 [details]
Patch to detect test cases without getResourcePath()
Caspar, please decide whether we want this. Around 35 test cases may need to be adjusted... Dude, we TOTALLY want this :-D But the check should make an exception for testcases that declare @NeedsCleanRepo, since they will work with a clean repo and therefore have no need to isolate their data into a test-specific resource. (And not only no need; also, no way -- otherwise they shouldn't be declaring @NeedsCleanRepo.) Patch coming up. Created attachment 193983 [details]
Patch to detect test cases without getResourcePath, v2
(In reply to comment #16) > Created attachment 193983 [details] > Patch to detect test cases without getResourcePath, v2 BTW, patch includes fixes for a few testcases, either making them use getResourcePath or declare @NeedsCleanRepo. Committed revision 7637: - trunk/plugins/org.eclipse.emf.cdo.tests Thanks, seems to work perfect now! Available in R20110608-1407 |