-
-
Notifications
You must be signed in to change notification settings - Fork 62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Show Junit test failures as ERROR diagnostics #665
Show Junit test failures as ERROR diagnostics #665
Conversation
@mfussenegger, Is this something that you would consider for review? Otherwise, I can close the issue and PR. Thanks |
@mfussenegger I think this PR adds great functionality! I don't know much Lua so can't fully review the PR myself, but I've been using this branch for two weeks with no issues at all. @vishal423, I've left a couple of comments based on my usage. |
@tleonardi, Glad! You liked the change. Irrespective of whether it gets merged over here, the change will reside on my fork as I need it for my day-to-day work. On a side note, I don't see your comments :) |
local lenses = lens.children or lens | ||
local failures = {} | ||
local error_symbol = '❌' | ||
local success_symbol = '✔️ ' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible to read these symbols from config so that they can be overridden?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may be possible, but, I haven't looked much into configuration aspect. Do you have different symbols in mind that provide better context as default?
lua/jdtls/junit.lua
Outdated
repl.append('✔️ ' .. test.method, '$') | ||
if start_line_num ~= nil then | ||
vim.api.nvim_buf_set_extmark(bufnr, ns, start_line_num, 0, { | ||
virt_text = { { '\t\t' .. success_symbol } }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it really necessary to add 2 tabs in front of the success_symbol? If you run the same test multiple times (e.g. with @RepeatedTest or @ParameterizedTest) you end up with a list of success_symbols taking up a lot of horizontal space. If you just print the success_symbol with no tabs at all the resulting list of success/error symbols is much tidier.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me take a look to see if we can optimize, but, I would still like to have 2 initial tabs before the symbol to differentiate it from the rest of the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recent commit should improve handling of Parameterized tests along with class level errors
@vishal423, the comments should now be visible. Thanks! |
7e81f4c
to
7077096
Compare
Closes #664
Test failures
Class initialization failure
Success scenario
Edit: Screenshots updated to display failure cause as error diagnostics