Skip to content

Fix the "Problem" messages for pytest failures when debugging.#15434

Merged
ericsnowcurrently merged 4 commits intomicrosoft:mainfrom
ericsnowcurrently:testing-results-diagnostic-message
Feb 19, 2021
Merged

Fix the "Problem" messages for pytest failures when debugging.#15434
ericsnowcurrently merged 4 commits intomicrosoft:mainfrom
ericsnowcurrently:testing-results-diagnostic-message

Conversation

@ericsnowcurrently
Copy link
Copy Markdown

fixes #14866

In #14311 we worked around a bug in a way that caused the strange message reported in #14866 (pytest with debugger). This change fixes that one without regressing the previous fix.

@ericsnowcurrently ericsnowcurrently added no-changelog No news entry required skip tests Updates to tests unnecessary labels Feb 18, 2021
@github-actions github-actions bot requested a review from karrtikr February 18, 2021 00:15
@github-actions github-actions bot requested a review from kimadeline February 18, 2021 00:15
Copy link
Copy Markdown

@kimadeline kimadeline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix looks good to me. Is there a reason why we need verbose explanations for it in comments?

Comment thread src/client/testing/common/managers/baseTestManager.ts Outdated
Comment thread src/client/testing/common/managers/baseTestManager.ts Outdated
Copy link
Copy Markdown

@karrtikr karrtikr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update existing tests if any. LGTM otherwise.

@ericsnowcurrently ericsnowcurrently changed the title Testing results diagnostic message Fix the "Problem" messages for pytest failures when debugging. Feb 18, 2021
@kimadeline
Copy link
Copy Markdown

Did you check that your new changes are not re-introducing #14067?

@ericsnowcurrently
Copy link
Copy Markdown
Author

Did you check that your new changes are not re-introducing #14067?

That was resolved with the changes in #15387. Test results with an undefined status will never reach the problematic code now. Furthermore, with this PR, diagPrefix will never be undefined now.

Also, I have manually verified that debugging tests (under pytest) does not fail (as in #14067) and no longer generates any entries in the "Problems" panel, whereas before (due to #14311) each test had an entry with the "Ok".

So the cause of #14067 no longer exists and #14866 is fixed.

@codecov-io
Copy link
Copy Markdown

Codecov Report

Merging #15434 (179731f) into main (22290e7) will decrease coverage by 0%.
The diff coverage is 35%.

@@          Coverage Diff           @@
##            main   #15434   +/-   ##
======================================
- Coverage     65%      65%   -1%     
======================================
  Files        561      561           
  Lines      26924    26954   +30     
  Branches    3901     3911   +10     
======================================
+ Hits       17521    17531   +10     
- Misses      8679     8695   +16     
- Partials     724      728    +4     
Impacted Files Coverage Δ
.../client/testing/common/managers/baseTestManager.ts 44% <0%> (-1%) ⬇️
src/client/testing/common/types.ts 93% <ø> (ø)
...sting/common/services/unitTestDiagnosticService.ts 80% <50%> (-16%) ⬇️
...nts/base/locators/composite/environmentsReducer.ts 94% <0%> (-3%) ⬇️
src/client/pythonEnvironments/base/locatorUtils.ts 58% <0%> (-2%) ⬇️
...ts/base/locators/composite/environmentsResolver.ts 94% <0%> (-2%) ⬇️
src/client/common/application/notebook.ts 8% <0%> (-1%) ⬇️
src/client/pythonEnvironments/base/locator.ts 81% <0%> (ø)
src/client/common/process/rawProcessApis.ts 10% <0%> (+<1%) ⬆️
...t/jupyter/languageserver/notebookConcatDocument.ts 5% <0%> (+<1%) ⬆️
... and 1 more

@ericsnowcurrently ericsnowcurrently merged commit df3d6c7 into microsoft:main Feb 19, 2021
@ericsnowcurrently ericsnowcurrently deleted the testing-results-diagnostic-message branch February 19, 2021 00:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog No news entry required skip tests Updates to tests unnecessary

Projects

None yet

Development

Successfully merging this pull request may close these issues.

When debugging pytest tests, extension displays "Ok" in the problems window

4 participants