bugfix: accommodate junit encoded test names #1411
Conversation
kasiaMarek
force-pushed
the
junit-encoded-names
branch
from
4308e78
to
d9a347f
Compare
| className: test._metalsParent.id, | ||
| tests: [test.label], | ||
| className: _metalsParent.id, | ||
| tests: [idStartsWithParent ? id.slice(parent.id.length + 1) : id], |
There was a problem hiding this comment.
When do we encode the test with name of the parent? Could we fix that on the server side?
There was a problem hiding this comment.
Do we still need the change here?
There was a problem hiding this comment.
Yeah, I asked you again for review kind of by accident (I revoked the request but I guess it already alarmed you). I forgot for a sec I was supposed to fixed that. :D
There was a problem hiding this comment.
Should be good now.
| @@ -94,8 +94,9 @@ function analyzeTestCases( | |||
|
|
|||
| if (parent) { | |||
| const tests = new Set(testCases.map((t) => t.id as TestName)); | |||
| const failed = Array.from(testCasesResults.values()) | |||
| .filter((result) => !tests.has(result.testName)) | |||
| const failed = Array.from(testCasesResults.entries()) | |||
There was a problem hiding this comment.
Could explain a bit about why these chanegs are needed? Also why do we need to drop the parent in the test case id? This doesn't seem related to the display name
There was a problem hiding this comment.
Oh, the changes in tests are not needed per se. We can go either way: test cases can have the parent's name encoded in the id or not. I understood you suggested that maybe it makes more sense not to do it: #1411 (comment), so I changed that. I didn't initially do it to avoid making more changes in the code.
One way or another for the names that come from the dap session we need to do a mapping, since depending on the build server the names might have the parent's id or not. Here the names returned by DAP were used (without mapping), which I believe to be an error anyway.
I'm only concerned with the id, because previously label (which now corresponds to the display name) was used when a test case was run.
| className: _metalsParent.id, | ||
| tests: [id], |
There was a problem hiding this comment.
| className: _metalsParent.id, | |
| tests: [id], | |
| className: test._metalsParent.id, | |
| tests: [test.id], |
that would be a bit less magical.
kasiaMarek
force-pushed
the
junit-encoded-names
branch
from
b6e5ca3
to
a14199c
Compare
use display name for test cases
connected to:
scalameta/metals#5485
scalameta/metals#5378