Skip to content

Fix precedence in parsePyTestModuleCollectionResult#4360

Merged
DonJayamanne merged 3 commits into
microsoft:masterfrom
tammoippen:fix-pytest-parsing
Feb 20, 2019
Merged

Fix precedence in parsePyTestModuleCollectionResult#4360
DonJayamanne merged 3 commits into
microsoft:masterfrom
tammoippen:fix-pytest-parsing

Conversation

@tammoippen
Copy link
Copy Markdown

@tammoippen tammoippen commented Feb 12, 2019 •
edited
Loading

For ...

  • Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR)
  • Title summarizes what is changing
  • Has a news entry file (remember to thank yourself!)
  • Has sufficient logging.
  • Has telemetry for enhancements.
  • Unit tests & system/integration tests are added/updated
  • Test plan is updated as appropriate
  • package-lock.json has been regenerated by running npm install (if dependencies have changed)

When discovering tests with pytest, the Python Test Log ended with:

Test Discovery failed: 
TypeError: Cannot read property 'item' of undefined

The developer console gives me:

[Extension Host] Python Extension: displayDiscoverStatus TypeError: Cannot read property 'item' of undefined
	at module.exports.l.parsePyTestModuleCollectionResult.t.forEach.t (/Users/tammoippen/.vscode/extensions/ms-python.python-2019.1.0/out/client/extension.js:77591:45)
	at Array.forEach (<anonymous>)
	at l.parsePyTestModuleCollectionResult (/Users/tammoippen/.vscode/extensions/ms-python.python-2019.1.0/out/client/extension.js:77527:11)
	at module.exports.l.getTestFiles.e.split.forEach (/Users/tammoippen/.vscode/extensions/ms-python.python-2019.1.0/out/client/extension.js:77463:23)
	at Array.forEach (<anonymous>)
	at l.getTestFiles (/Users/tammoippen/.vscode/extensions/ms-python.python-2019.1.0/out/client/extension.js:77458:29)
	at l.parse (/Users/tammoippen/.vscode/extensions/ms-python.python-2019.1.0/out/client/extension.js:77445:24)
	at h.<anonymous> (/Users/tammoippen/.vscode/extensions/ms-python.python-2019.1.0/out/client/extension.js:77387:31)
	at Generator.next (<anonymous>)
	at s (/Users/tammoippen/.vscode/extensions/ms-python.python-2019.1.0/out/client/extension.js:77303:21)

Which pointed me to parsePyTestModuleCollectionResult and parentNode not being correctly guarded (undefined in my case).

@msftclas
Copy link
Copy Markdown

msftclas commented Feb 12, 2019 •
edited
Loading

CLA assistant check
All CLA requirements met.

@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 12, 2019 •
edited
Loading

Codecov Report

Merging #4360 into master will increase coverage by 22%.
The diff coverage is 100%.

@@           Coverage Diff            @@
##           master   #4360     +/-   ##
========================================
+ Coverage      56%     78%    +22%     
========================================
  Files         361     439     +78     
  Lines       15274   20515   +5241     
  Branches     2373    3288    +915     
========================================
+ Hits         8494   15805   +7311     
+ Misses       6236    4706   -1530     
+ Partials      544       4    -540
Flag Coverage Δ
#Linux 66% <100%> (?)
#Windows 66% <100%> (?)
#macOS 66% <100%> (?)

@tammoippen
Copy link
Copy Markdown
Author

tammoippen commented Feb 19, 2019

After tempering with my tests, I think the issue occurs with newer pytests and tests with multiline docstrings and using verbose output. See here aswell: pytest-dev/pytest#4582. I think this breaks the parser?

@DonJayamanne DonJayamanne merged commit e708760 into microsoft:master Feb 20, 2019
@tammoippen tammoippen deleted the fix-pytest-parsing branch February 25, 2019 16:31
@lock lock Bot locked as resolved and limited conversation to collaborators Jul 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@DonJayamanne DonJayamanne DonJayamanne approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

2019, week 7 - Feb Sprint 4

Development

Successfully merging this pull request may close these issues.

4 participants

@tammoippen @msftclas @DonJayamanne @brettcannon