Skip to content
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

Test failure should not be reported as a compile error #9640

Closed
isidorn opened this issue Jan 17, 2020 · 7 comments · Fixed by #16769
Closed

Test failure should not be reported as a compile error #9640

isidorn opened this issue Jan 17, 2020 · 7 comments · Fixed by #16769
Assignees
Labels
area-testing feature-request Request for new features or functionality needs proposal Need to make some design decisions partner ask verified Verification succeeded
Milestone

Comments

@isidorn
Copy link

isidorn commented Jan 17, 2020

Have a test failing.
Notice that python reports this as an Error. This shows up in the Problems panel and with a squigle.
This is not the right thing to do. Test errors should not be reported as compile errors.

I am not sure if there is a better alternative to use
fyi @jrieken @luabud

Also we now support rendering emojis so test failures could be rendered a bit better. @jrieken can provide more details.

@isidorn isidorn added triage-needed Needs assignment to the proper sub-team feature-request Request for new features or functionality labels Jan 17, 2020
@luabud luabud added needs decision area-testing and removed triage-needed Needs assignment to the proper sub-team labels Jan 17, 2020
@luabud
Copy link
Member

luabud commented Jan 17, 2020

FWIW this was implemented by an external contributor for a feature request: #120, PR: #3303.

If I'm not mistaken the Problems window was used because there isn't a way to jump to the failed lines through the Output window?! But I may be wrong.

Anyway, we should then look into a better solution to solve the problem other than using the Problems window. As you pointed out, this isn't a syntax or compilation error.

@DonJayamanne
Copy link

DonJayamanne commented Jan 19, 2020

IMHO I think the problems window is fine.
Though reporting test issues in problems window while we have a test explorer (dedicated UI for tests) does seem awkward.

The user requirement is that when tests fail, they expect to see the failures and be able to jump to the failed lines easily. Currently this isn't possible from the test explorer.
I.e. the test explorer doesn't provide a view for failures in a list. Agreed we can provide a custom view. But users wouldn't know about such a toggle, or we'd need to create another UI list view with exact same functionality as problems window, however that doesn't seem right.

Test errors should not be reported as compile errors.

Not sure this is correct. Today we have spell checkers reporting errors in problems window and they aren't compile errors. I wouldn't be surprised if there were others.
That may have been the intent, however I believe the community is now using the problems window to surface other types of problems.

@luabud
Copy link
Member

luabud commented Feb 26, 2020

@isidorn I talked to the team about it and the sentiment was that we always thought having that report in the problems window would be fine (after all, having failed tests in your project could be seen as a problem).

Are there any guidelines from VS Code that we could refer to in terms of what should or shouldn't be displayed in the Problems window?

I think Don raised a good point that the community may be using the Problems window to surface various types of problems, so having some guidelines could help on this.

@isidorn
Copy link
Author

isidorn commented Feb 27, 2020

Adding @jrieken and @sandy081 for Prolbems view guidelines

@sandy081
Copy link
Member

Problems view is for code diagnostics which are generated by a compiler and Tests view is for managing and running tests. Former is compile time and later is during run time. I think this is the standard in general everywhere. As always, we never enforced this but it is expected that extensions follow this so that users will get consistent experience.

image

image

Agreed that before having tests view, authors were using different approaches (output, problems, code-decorations) to show tests. But after introducing this, we recommended all to create a test view and show test results there. You can define your own actions to run, debug the tests and more which you cannot do from Problems view where you can only do quick fixes (which is to fix your code to compile).

Today we have spell checkers reporting errors in problems window and they aren't compile errors. I wouldn't be surprised if there were others.

I do not think I agree with this and they are compile time errors.

@jrieken
Copy link
Member

jrieken commented Feb 27, 2020

The problems view is for syntax or semantic errors and for lint'ing messages. You can obviously bend the definition of a "problem" but when we design and plan features for diagnostics then we do not think about test running, e.g "Run & Debug" will prompt you in the presence of diagnostics - that makes sense for compile errors but not for test errors, esp. not when "run" means "re-run the tests". That means there is a chance of future (semantic) breakage and for non-python users this might be confusing

Still, we like the idea of showing test failures in-line with sources (we just don't like the use of diagnostics for this) and this issue should also be an invitation to brain storming how VS Code (as a platform) should offer better test reporting UX and API. The Jest extension, while also using diagnostics 😞, uses line-end decorations which I think is pretty cool.

Screenshot 2020-02-27 at 11 22 38

So, lets think further than the test explorer and lets start to collect requirements and ideas for better test integration

@luabud luabud added needs proposal Need to make some design decisions and removed needs decision labels Feb 27, 2020
@luabud luabud self-assigned this Aug 12, 2020
@karthiknadig
Copy link
Member

Fixed via #16769

@karthiknadig karthiknadig added the verified Verification succeeded label Jul 29, 2021
@karthiknadig karthiknadig added this to the August 2021 milestone Aug 5, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-testing feature-request Request for new features or functionality needs proposal Need to make some design decisions partner ask verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants