Bug 154552 - move model tests to use a create and compare strategy
Summary: move model tests to use a create and compare strategy
Status: RESOLVED FIXED
Alias: None
Product: AspectJ
Classification: Tools
Component: Compiler (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows XP
: P3 enhancement (vote)
Target Milestone: 1.5.3   Edit
Assignee: aspectj inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-08-21 11:30 EDT by Helen Beeken CLA
Modified: 2012-04-03 16:00 EDT (History)
0 users

See Also:


Attachments
moving existing model tests to use new structure (18.73 KB, application/zip)
2006-08-21 11:36 EDT, Helen Beeken CLA
no flags Details
patch to remove unwanted tests (11.44 KB, patch)
2006-08-22 05:44 EDT, Helen Beeken CLA
aclement: iplog+
Details | Diff
adding a comment to AsmManager.reportModelInfo (946 bytes, patch)
2006-08-24 05:47 EDT, Helen Beeken CLA
aclement: iplog+
Details | Diff
patch to ensure model tests work on linux (1.62 KB, patch)
2006-08-25 04:38 EDT, Helen Beeken CLA
aclement: iplog+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Helen Beeken CLA 2006-08-21 11:30:08 EDT
A comment in bug 141730:

"Also I'm concerned about how bloated all the tests are that verify anything to
do with the structure model.  If we need to do much more of this they should
move to a create and compare strategy, like the weave messages tests - where we
store the expected output on disk.  Normally running the test creates some
output and compares it with the output on disk reporting discrepancies, but the
tests can also be run in a mode that regenerates the expected output."

Raising this enhancement to cover this.

The infrastructure has already been committed as part of comment #11 of bug 145963, however the existing tests should be moved over to the new structure.
Comment 1 Helen Beeken CLA 2006-08-21 11:36:22 EDT
Created attachment 48298 [details]
moving existing model tests to use new structure

This zip file contains the following:

- modelTests-ajdt-core.txt: apply to the org.aspectj.ajdt.core project. This adds a call to report the model info if there are compilation errors. This is used in one of the model tests which uses declare error.

- modelTests-tests.txt: apply to the tests project.

- aspectpath.jar: put in tests\model\pr141730_3 if this jar was corrupted in the modelTests-tests.txt patch

- inpath.jar: put in tests\model\pr141730_4 if this jar was corrupted in the modelTests-tests.txt patch


Together these patches rewrite the existing model tests to fit in with the new structure committed as part of bug 145963. 

Note that the tests use the JDTLikeHandleProvider from bug 141730 as these handles are consistent over different test runs on different os.
Comment 2 Andrew Clement CLA 2006-08-22 04:21:35 EDT
patches are in.  I know the JDTlike handle provider makes things easier to test - so we really must make it the default if tests rely on it, otherwise we aren't testing what AJDT will be doing.  Has it been activated in AJDT yet?  We are waiting for confirmation that it is fine there before making it the default in AJ.
Comment 3 Helen Beeken CLA 2006-08-22 05:44:19 EDT
Created attachment 48364 [details]
patch to remove unwanted tests

Apply this patch to the tests project.

This patch removes all the unwanted files/tests which are no longer required since moving the model tests.
Comment 4 Andrew Clement CLA 2006-08-22 06:30:27 EDT
patches committed.
Comment 5 Helen Beeken CLA 2006-08-23 04:02:27 EDT
In response to "Has it been activated in AJDT yet?" the answer is "no". I'm just working on running the AJDT tests with the JDTLikeHandleProvider and will add comments/patches regarding this to that enhancement (bug 141730).
Comment 6 Helen Beeken CLA 2006-08-23 11:49:21 EDT
Some comments on the model tests....

1. If the format of the reporting in AsmManager changes then the tests will fail in their comparison. The files against which to compare can be regenerated, however, the tests currently assume that the reporting dumps out both the model and the relationships and consequently tests both at the same time. If the formatting changes then it is necessary for both to still be tested

2. As mentioned, the JDTLikeHandleProvider has been used to ensure compatibility across different test runs and os. The one thing to look out for is the numbering. Currently, if there is more than one before advice in an aspect then they are numbered xxxx$1 and xxxx$2. The “$1” and “$2” are used in the handles. It is assumed that this is consistent over different test runs (note it is the byteCodeName which is used to get the count number)
Comment 7 Helen Beeken CLA 2006-08-24 03:45:36 EDT
One thought about the consistency is to use sourcelocations rather than handles in the relationship map part of the test. This would require adding another IModelFilter method to process the handle. Unfortunately, the implementation of this method in TestFilter would be to get the hierarchy, ask it for the ipe corresponding to the given handle and then ask this ipe for its sourcelocation (which would then need to be processed to have "TEST_SANDBOX" rather than the absolute path). This is on the way to being back where we started with the walking of the hierarchy. The one difference being that its all hidden within the test infrastructure rather than each test having to write it all out individually.
Comment 8 Andrew Clement CLA 2006-08-24 04:04:00 EDT
I was surprised that the existing debug dump mechanism was used for creating the expected output - if the dump format changes then, as you say, the tests will break.  But as long as we know the dump format is important, we won't make unnecessary changes to it.

On the problem with the relationships comparison - is it not intended that the JDT handles will be highly robust things that can be relied upon?  Returning to source locations again would seem like a step backwards...
Comment 9 Helen Beeken CLA 2006-08-24 05:47:00 EDT
Created attachment 48566 [details]
adding a comment to AsmManager.reportModelInfo

Apply this patch to the asm project.
Comment 10 Andrew Clement CLA 2006-08-24 05:49:51 EDT
patch from comment #9 committed.
Comment 11 Helen Beeken CLA 2006-08-24 05:56:57 EDT
All work to be done on this has been committed, therefore closing as fixed.
Comment 12 Helen Beeken CLA 2006-08-25 04:38:15 EDT
Created attachment 48693 [details]
patch to ensure model tests work on linux

Currently testNewIProgramElementMethods_pr141730() does't work on linux as the directory is PR141730_1 and it's looking for pr141730_1. This patch fixes this by renaming the directory (since the expected file needs the same name as the directory).

Apply this patch to the tests project.
Comment 13 Andrew Clement CLA 2006-08-25 05:53:02 EDT
that kind of patch doesnt apply nicely on windows - at least it didn't for me.  I've done the rename manually, deleting the folder, committing the delete, recreating it, committing the recreate - please try it on Linux now.
Comment 14 Helen Beeken CLA 2006-08-25 06:36:29 EDT
Yes, this is now working on Linux.