Show error message with details about step validation#2837
Show error message with details about step validation#2837jensakejohansson wants to merge 3 commits into
Conversation
…n - show it. Signed-off-by: jensakejohansson <jens.johansson@systemverification.com>
Signed-off-by: jensakejohansson <jens.johansson@systemverification.com>
|
Hopefully fixes failing tests: getgauge/gauge-tests#153 The errors messages differs between the language runners and might need to be improved in places, but I still believe it's a good idea to show them in the IDE. They can always be improved later on. |
|
I've merged the test change just to move forward with the tests here but I wonder
Do you happen to know if there are other places currently where such messages can be multi-lined? |
Signed-off-by: jensakejohansson <jens.johansson@systemverification.com>
|
Yea, I revised this as follows: Insight: Message() is only used by the IDE LSP diagnostics path, while Error() is used by console, reports, plugins, skip errors, and all other display paths. So what about we add the detail to Message() only? That would address the concern about console output, which I believe to be important. The output may become too noisy. Especially since, at first glance, it doesn't seems to have been a coherent approach to what message content different language implementations should provide. Also, no need to update any tests (unit or the integration tests). Still, when developing in an IDE you can get some extra useful information from the runner. Yes, one can argue that error messages becomes inconsistent across "environments" - but to me it makes some sense. Any opinions? |
|
A PR that should make the LSP tests pass. Check only the first line in message - indicating the error type - but let details-part slip through.... P.S |
Assuming you’re on Windows the LSP tests are totally unusable on Windows for some reason. I have no idea why. Windows is slow, yes, but they take 5-6 times longer than Linux or MacOS which is insane. |
Benchmark Results
Notes
See Workflow log for more details. |
im not sure how important the console log format is, since i dont tend to use the validate workflow in normal operation Do you still get the extended detail information in the lsp runner and gauge log files when working with an IDE even if we don’t include in the console output? If so, that may be sufficient. Other than that @sriv or @zabil may be able to give some insight into general logging methodology, and the wisdom of including line breaks in either messages returned for console log or the IDE LsP responses. |
That can be achieved by logging in the language runner. I added this to getgauge/gauge-dotnet#287 Logs at debug-level in the runner it will not be routed to the console out by default, since default log-level is set to info. If you run Gauge CLI with log-level explicitly set to debug it will of course show up in console together with a lot of other logging that is normally not shown. Example of output: gauge.log example: lsp.log example: console (at default log-level): And of course message to IDE. |
Related to: getgauge/gauge-dotnet#287
When a runner (e.g. gauge-dotnet) detects a duplicate step implementation, it returns a StepValidateResponse with both an errorType (DUPLICATE_STEP_IMPLEMENTATION) and an errorMessage containing details like file paths and line numbers of the conflicting implementations.
However, Gauge ignores the errorMessage field, the displayed message is only derived from the enum name DUPLICATE_STEP_IMPLEMENTATION → "Duplicate step implementation", discarding any additional context the runner provided.
Fix:
After deriving the base message from the error type append the runner's errorMessage if it is non-empty.
This is backwards-compatible with all runners. errorMessage is a string field in the protobuf contract — if a runner doesn't set it, GetErrorMessage() returns "" and the behavior is unchanged.
Example:
Before:
Duplicate step implementation
Now:
Duplicate step implementation
Step : Click {}
ActionsA.Click in ActionsA.cs:15
ActionsB.Click in ActionsB.cs:42