Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

test: increase coverage for worker_threads #22942

Closed
wants to merge 1 commit into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Sep 19, 2018

Provide a test to cover adding setting onmessage to a non-function.

This provides previously-missing coverage for an else block in the
onmessage setter.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

@Trott Trott added test Issues and PRs related to the tests. worker Issues and PRs related to Worker support. labels Sep 19, 2018
@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Sep 19, 2018
@Trott
Copy link
Member Author

Trott commented Sep 19, 2018

Provide a test to cover adding setting `onmessage` to a non-function.

This provides previously-missing coverage for an else block in the
`onmessage` setter.
@Trott
Copy link
Member Author

Trott commented Sep 19, 2018

Pushed changes to add more comments explaining the test.

CI: https://ci.nodejs.org/job/node-test-pull-request/17320/

@Trott
Copy link
Member Author

Trott commented Sep 19, 2018

@Trott Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 19, 2018
@Trott
Copy link
Member Author

Trott commented Sep 19, 2018

@Fishrock123
Copy link
Contributor

${STATUS_LABEL} — tests failed

Interesting...

@joyeecheung
Copy link
Member

@Fishrock123 See nodejs/build#1452

@Trott
Copy link
Member Author

Trott commented Sep 21, 2018

Landed in c7bf02a

@Trott Trott closed this Sep 21, 2018
Trott added a commit to Trott/io.js that referenced this pull request Sep 21, 2018
Provide a test to cover adding setting `onmessage` to a non-function.

This provides previously-missing coverage for an else block in the
`onmessage` setter.

PR-URL: nodejs#22942
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
targos pushed a commit that referenced this pull request Sep 23, 2018
Provide a test to cover adding setting `onmessage` to a non-function.

This provides previously-missing coverage for an else block in the
`onmessage` setter.

PR-URL: #22942
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. test Issues and PRs related to the tests. worker Issues and PRs related to Worker support.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants