FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test: move AEAD test vectors out of script by tniessen · Pull Request #21873 · nodejs/node · GitHub

/ node Public

test: move AEAD test vectors out of script - #21873

Closed
tniessen wants to merge 2 commits into
nodejs:masterfrom
tniessen:test-refactor-aead-test-vectors
ConversationCommits (2)ChecksFiles changed
Closed

test: move AEAD test vectors out of script#21873
tniessen wants to merge 2 commits into
nodejs:masterfrom
tniessen:test-refactor-aead-test-vectors

Conversation

tniessen commented Jul 18, 2018

Copy link
Copy Markdown
Member

This reduces the test by almost 700 lines (more than 50%!). This would optimally be a JSON file, but we should keep the comments, so it remains JS for now.

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

nodejs-github-bot commented Jul 18, 2018

Copy link
Copy Markdown
Collaborator

@tniessen build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/308/pipeline

nodejs-github-bot added the test Issues and PRs related to the tests. label Jul 18, 2018
lpinca reviewed Jul 18, 2018
View reviewed changes
Comment thread test/fixtures/aead-vectors.js Outdated
tag: 'd0c515f4d1cdd4fdac4f02ab',
tampered: true
}
]; No newline at end of file

lpinca Jul 18, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Nit: missing new line :)

tniessen Jul 19, 2018

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Oh right, Windows...

maclover7 commented Jul 20, 2018

Copy link
Copy Markdown
Contributor

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

maclover7 added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Jul 20, 2018

tniessen commented Jul 21, 2018

Copy link
Copy Markdown
Member Author

Thanks @maclover7! Whatever node-test-commit-custom-suites is, it failed... Resumed in https://ci.nodejs.org/job/node-test-pull-request/15964/

tniessen commented Jul 22, 2018

Copy link
Copy Markdown
Member Author

Landed in b2ac3af.

tniessen closed this Jul 22, 2018
tniessen added a commit that referenced this pull request Jul 22, 2018
test: move AEAD test vectors out of script
b2ac3af 
PR-URL: #21873
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>

maclover7 commented Jul 22, 2018

Copy link
Copy Markdown
Contributor

Whatever node-test-commit-custom-suites is, it failed...

node-test-commit-custom-suites lets you plug in different test suites to run. Looks like @Trott is working on documenting this in #21927, but I'll just add on to say that it's used to run the default test suites with the --worker flags for all node-test-commit builds, to make sure we don't break stuff with workers

targos commented Jul 24, 2018

Copy link
Copy Markdown
Member

Depends on #21447 to land on v10.x-staging

targos added backport-requested-v10.x and removed author ready PRs that have at least one approval, no outstanding review comments, and a CI started. labels Jul 24, 2018
targos pushed a commit that referenced this pull request Aug 29, 2018
test: move AEAD test vectors out of script
09a824f 
PR-URL: #21873
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
targos pushed a commit that referenced this pull request Sep 3, 2018
test: move AEAD test vectors out of script
4cdc61b 
PR-URL: #21873
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
targos mentioned this pull request Sep 5, 2018
Merged
targos pushed a commit that referenced this pull request Sep 6, 2018
test: move AEAD test vectors out of script
a972159 
PR-URL: #21873
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
This was referenced Sep 6, 2018
Closed
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell approved these changes
lpinca lpinca approved these changes
trivikr trivikr approved these changes
+1 more reviewer
maclover7 maclover7 approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

test Issues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

Back | FazBrowse Home | New Git URL