Skip to content
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

test: handle app already signed error #34331

Merged
merged 1 commit into from Jul 22, 2020
Merged

test: handle app already signed error #34331

merged 1 commit into from Jul 22, 2020

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Jul 13, 2020

In the GitHub Actions CI, test-macos-app-sandbox.js can fail due
to the application already being signed. This commit updates
the test to handle that condition.

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

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Jul 13, 2020
@cjihrig
Copy link
Contributor Author

cjihrig commented Jul 13, 2020

This is the wrong approach. I'm going to update this PR to try passing the --force option to the codesign executable.

@richardlau
Copy link
Member

I'm kind of confused on how the app can be already signed. Aren't the actions run in a pristine environment? (The long build times would suggest so.)

@cjihrig
Copy link
Contributor Author

cjihrig commented Jul 13, 2020

It wasn't clear to me how the app was already signed either. To be honest, I didn't try to figure it out either, as fixing the test seemed more straightforward than diving into the GitHub Actions setup.

@cjihrig cjihrig force-pushed the macos branch 2 times, most recently from 2cd6562 to deeadb7 Compare July 14, 2020 23:39
Copy link
Member

@tniessen tniessen left a comment

Choose a reason for hiding this comment

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

RSLGTM

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 21, 2020

In the GitHub Actions CI, test-macos-app-sandbox.js can fail due
to the application already being signed. This commit updates
the test to handle that condition.

Refs: nodejs#33944
PR-URL: nodejs#34331
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@cjihrig cjihrig merged commit e7ed066 into nodejs:master Jul 22, 2020
@cjihrig cjihrig deleted the macos branch July 22, 2020 19:11
cjihrig added a commit that referenced this pull request Jul 23, 2020
In the GitHub Actions CI, test-macos-app-sandbox.js can fail due
to the application already being signed. This commit updates
the test to handle that condition.

Refs: #33944
PR-URL: #34331
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
MylesBorins pushed a commit that referenced this pull request Jul 27, 2020
In the GitHub Actions CI, test-macos-app-sandbox.js can fail due
to the application already being signed. This commit updates
the test to handle that condition.

Refs: #33944
PR-URL: #34331
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@ruyadorno ruyadorno mentioned this pull request Jul 28, 2020
addaleax pushed a commit that referenced this pull request Sep 22, 2020
In the GitHub Actions CI, test-macos-app-sandbox.js can fail due
to the application already being signed. This commit updates
the test to handle that condition.

Refs: #33944
PR-URL: #34331
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
addaleax pushed a commit that referenced this pull request Sep 22, 2020
In the GitHub Actions CI, test-macos-app-sandbox.js can fail due
to the application already being signed. This commit updates
the test to handle that condition.

Refs: #33944
PR-URL: #34331
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@codebytere codebytere mentioned this pull request Sep 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants