Skip to content

Smoke tests in nodejs#6499

Closed
DonJayamanne wants to merge 112 commits intomicrosoft:masterfrom
DonJayamanne:eurekaSmokey
Closed

Smoke tests in nodejs#6499
DonJayamanne wants to merge 112 commits intomicrosoft:masterfrom
DonJayamanne:eurekaSmokey

Conversation

@DonJayamanne
Copy link
Copy Markdown

For #5912

  • Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR)
  • Title summarizes what is changing
  • [n/a] Has a news entry file (remember to thank yourself!)
  • [n/a] Appropriate comments and documentation strings in the code
  • Has sufficient logging.
  • [n/a] Has telemetry for enhancements.
  • [n/a] Unit tests & system/integration tests are added/updated
  • [n/a] Test plan is updated as appropriate
  • package-lock.json has been regenerated by running npm install (if dependencies have changed)
  • [n/a] The wiki is updated with any design decisions/details.

@DonJayamanne DonJayamanne added the no-changelog No news entry required label Jul 8, 2019
Copy link
Copy Markdown

@rchiodo rchiodo left a comment

Choose a reason for hiding this comment

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

:shipit:

Comment thread package.json
Comment thread src/smoke/README.md
Copy link
Copy Markdown

@ericsnowcurrently ericsnowcurrently left a comment

Choose a reason for hiding this comment

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

Here's my initial review. It covers everything except:

  • src/smoke/src/
  • uitests/

Also:

  • presumably src/smoke/vscode/ is the code we copied and should not be reviewed. Is that right?
  • why are we keeping a .vsix in the repo (why not rebuild it where needed) and how are we going to keep it up to date?
  • I'm not a fan of using the term "smoke tests" here. Smoke tests is the minimal subset of your full test suite that you can (relatively) quickly run to sanity-check the code base. This matters more for test suites that run on the order of days (or many hours). I'd expect the tests you've added to be called "ui tests". If some of them are also smoke tests, that's fine, but the whole set shouldn't be called "smoke tests".

Comment thread .vscode/launch.json
Comment thread .vscode/settings.json
Comment thread .vscode/settings.json
Comment thread build/ci/templates/uitest_jobs.yml
Comment thread build/ci/templates/uitest_phases.yml Outdated
Comment thread src/smoke/README.md Outdated
Comment thread src/smoke/bootstrap/README.md Outdated
Comment thread src/smoke/bootstrap/extension/extension.js
Comment thread src/smoke/bootstrap/extension/extension.js
Comment thread tsconfig.json
@lock lock Bot locked as resolved and limited conversation to collaborators Jul 29, 2019
@DonJayamanne DonJayamanne deleted the eurekaSmokey branch September 29, 2019 21:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

no-changelog No news entry required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants