Relax check file match with stack trace#22389
Conversation
66cb2b1 to
e6fa472
Compare
e6fa472 to
f30a2d9
Compare
f30a2d9 to
fadedc6
Compare
fadedc6 to
d7e627b
Compare
d7e627b to
7d3e807
Compare
7d3e807 to
bc50716
Compare
bc50716 to
09dc35c
Compare
SolalPirelli
left a comment
There was a problem hiding this comment.
If the specific goal is macro-related code, should this be enabled only for those? IMHO ignoring all line numbers is risky and might hide unrelated bugs...
|
In my experience, testing stack traces by default is noise. The tweak only relaxes the source line, which changes with arbitrary edits. If I want to test a stack trace or positions, I would do that with a predicate that explains what property of the stack elements is being enforced. The current situation is poor isolation of tests. By contrast, a tweak to tree positions will annoyingly but helpfully require updates to tests. (Edit: similarly for the inlined positions shown for errors, which are a feature.) I think the fix for the directive in tests may have got in already. (Edit: no, that's a different fix!) |
09dc35c to
a55e72b
Compare
a55e72b to
60b5307
Compare
A handful of tests under neg-macros include a stack trace in the check file, which makes them sensitive to irrelevant changes in distant source code.
This commit conservatively ignores line numbers in stack trace output when doing line text comparisons of test output.
A future improvement would be for
suppressAllOutputto be a filter that can suppress output or normalize it on a per-need basis.The test group name is corrected to
negMacros.As shown at 3cff3be
did not previously turn off the options, as it did for
scala-cli. It's not immediately obvious whether it's a bug that the test requires the objects (which are removed for other tests in that commit).Now,
//>must be at the beginning of the line. (There may be other ways the test syntax could be aligned with CLI directives.)