-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Update parser for pytest to handle changes in pytest >= 3.7 (while still supporting pytest < 3.7) #2465
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
Conversation
| const trimmedLine: string = line.trim(); | ||
|
|
||
| if (trimmedLine.startsWith('<Package \'')) { | ||
| // process the previous lines |
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.
"Process the previous lines."
| private parsePyTestModuleCollectionResult(rootDirectory: string, lines: string[], testFiles: TestFile[], parentNodes: { indent: number; item: TestFile | TestSuite }[]) { | ||
|
|
||
| /** | ||
| * Extract the 'package' name from a given PyTest (>= 3.7) output line |
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.
Missing a period.
| runner.verifyAll(); | ||
| testParser.verifyAll(); | ||
| }); | ||
| // build tests for the test data that is relevant for this platform. |
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.
"Build"
| }); | ||
| // build tests for the test data that is relevant for this platform. | ||
| pytestScenarioData.forEach((testScenario) => { | ||
| const testPlatformType: PytestDataPlatformType = |
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.
Does this need to be re-calculated for each test scenario?
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.
Nope, fixed.
|
|
||
| // tslint:disable-next-line:no-invalid-this | ||
| if (testPlatformType === testScenario.platform) { | ||
| const testDescription: string = 'PyTest '.concat('[', testScenario.pytest_version_spec, '] ', testScenario.description); |
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.
Any reason not to use string interpolation?
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.
Nope, changed.
3e7d698 to
8a6e6b7
Compare
Fix microsoft#2347 - Add the 'Package' information to the Parser - Correct how we discover mixed package/non-package tests - Add unit tests for < 3.7.0 parsing - Add unit tests for >= 3.7.0 parsing
- add json test data for running through various scenarios (WIP)
- Better for coping with many scenarios - Update tests to pull scenario data from json file - Linux/macOS scenarios are covered - TODO: Windows
- Discovery and parsing are related but belong in separate files
8a6e6b7 to
7dbf4db
Compare
- remove some unnecessary operations from for loop. - correct string construction to use interpolation.
Fixes #2347
Add the 'Package' information to the Parser
Correct how we discover mixed package/non-package tests
Add unit tests for < 3.7.0 parsing
Add unit tests for >= 3.7.0 parsing
Title summarizes what is changing
Has a news entry file (remember to thank yourself!)
Has unit tests & system/integration tests
Any new/changed dependencies inpackage.jsonare pinned (e.g."1.2.3", not"^1.2.3"for the specified version)package-lock.jsonhas been regenerated by runningnpm install(if dependencies have changed)WIP - Additional tasks to complete