Hi Daniel,
Thanks for reviewing and suggestions, 1> add big warning to warn
maintainer that the tests were dependent on line numbers.; 2> verify
line numbers by parsing the source code. I think your second suggestion
is great! :-), but to keep the logic simple and straight, I prefer your
first suggestion which is to add big warning. When tests detect that
line numbers are mismatched, there will be detailed error message
explaining the reason.
webrev : http://cr.openjdk.java.net/~mli/8144321/webrev.01/
Thank you
-Hamlin
On 2015/12/5 18:35, Daniel Fuchs wrote:
HI Hamlin,
If I understand well any line, comment, etc, added to the test
in the future is likely to make the tests fail because the line numbers
will no longer match.
I have seen other tests (I think they where compiler tests) which
exhibited such a characteristic, but those had a big warning in
their summary to warn maintainer that the tests were dependent
on line numbers.
If you can't find a way to make these test independent of
line numbers, then at least there should be a big warning
in the test summary, together with instruction on how to figure out
which values to put in the annotation...
One possibility to avoid having the test depend on hardcode line numbers
could be to embed some logic in the test to make it process (read) its
own source
file to discover the actual line numbers at run time and generate some
kind of
data file/or static Map that StackFrameAnnotationUtil could in turn
read to get
the expected line number.
For instance, instead of embedding the real number, the annotation could
embed an index, and the corresponding line could have a marker comment
at the end referring to the index... e.g.:
93 @StackFrameAnnotation(lineNumbers = {1, 2})
...
120 walker.walk(s -> { // should check this line number in
@StackFrameAnnotation. // @LINE#1
121 s.filter(f -> f.getClassName().startsWith("AcrossThreads$"))
122 .forEach(f -> StackFrameAnnotationUtils.verifyLineNumber(
123 f.getClassName(),
124 f.getMethodName(),
125 f.getLineNumber().getAsInt()));
126 return null;
127 });
128 } else {
129 assertWalker(walker, n); // should check this line number in
@StackFrameAnnotation. // @LINE#2
130 }
The test could then first process its source file to figure out that @LINE#1 is
at line 120 and
@LINE#2 is at line 129, and then build a map { 1 -> 120, 2 -> 129 } which
StackFrameAnnotationUtil would use to get the real line number from the index
it finds in the annotation...
Directly hardcoding line numbers inside annotations in the test looks
a bit fragile. Maybe someone else on the list will be able to suggest
even better ideas :-) best regards, -- daniel On 12/5/15 6:00 AM,
Hamlin Li wrote:
Hi all, Would you please help to review the test development of
https://bugs.openjdk.java.net/browse/JDK-8144321. webrev :
http://cr.openjdk.java.net/~mli/8144321/webrev.00/ Thank you -Hamlin
