feat: Display line number of failing test in pretty formatter #1615
Conversation
acoulton
left a comment
There was a problem hiding this comment.
@jdeniau I see what you mean about the implementation and trying to match the scenario and the step. I agree it feels a bit uncomfortable just matching on the array index.
AFAICS, although Statistics::getFailedSteps() is typed as returning StepStat[] for BC reasons, in practice we only ever create / register instances of StepStatV2. And that has a getScenarioPath() which I think should always match the getPath() of ScenarioStat.
So maybe it's possible to search through the $stepStats array looking for one that is an instanceof StepStatV2 and has a matching ->getScenarioPath()? That is possibly cleaner / more robust than matching on the array indices?
On the translations, I believe that @carlos-granados has asked ChatGPT to generate the translations for new strings he's added recently. They seemed to be plausible, for languages we could speak / vaguely recognise 😆
Yes, what @acoulton proposes seems more robust, please explore that. And he is right that I asked ChatGPT for recent translations, so I think we can do the same in this case. |
|
@jdeniau could you rebase and resolve conflicts? Thanks |
|
@jdeniau do you think you'll have time to get back to this? If not I'd be happy to pick it up from here - would be good to get it merged. |
|
To be honest, I don't think when I could get back to this and I don't know remember if those are easy changes. |
94db118 to
323905c
Compare
|
@acoulton Well I'm working on it right now finally ! |
26701f3 to
f2994ee
Compare
cf6ef52 to
69d4131
Compare
bd6df3c to
6354db3
Compare
6354db3 to
0345e96
Compare
6891dd7 to
41e5911
Compare
|
@acoulton Everything should be OK 🎉 |
|
@jdeniau this is looking good, thanks for your good work, but there are several failed tests in the Windows CI tester where instead of the line number it prints the path of the feature file. I think that this is probably a problem of the |
|
@carlos-granados yes I saw that! Nice catch. I will fix that soon. There are some weird line number reported too (before the scenario, but it may be OK I need to check) |
|
Everything should be fine now. About the reported lines before the scenario, it's because we are on a Scenario Outline with examples, so the step is before the reported line, which is OK. |
carlos-granados
left a comment
There was a problem hiding this comment.
Looking all good, thanks @jdeniau
Fixes #1612
Other possible unused option
I hesitated to use the
printStepListused in the Progress printer, but it would have display two time the same informations.Implementation
I am not really fan of my implementation, because the failing scenario is not linked to the failing steps.
I did assume that, as the rest of the steps in the scenario are just skipped, there would be a match between the
failingScenariosStatsand thefailingStepsStats, but I would have been far more confident if I could do something like$failingStep->getScenario()to find the failing step(s?) of a scenario.I18n
I do not know what are the translation process, so I did only translate in english and in french, but it will probably display something wrong in other languages (unless there is a fallback on english ?)