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

big fix for CI #40684

Merged
merged 0 commits into from
Nov 1, 2021
Merged

big fix for CI #40684

merged 0 commits into from
Nov 1, 2021

Conversation

Trott
Copy link
Member

@Trott Trott commented Nov 1, 2021

This incorporates #40670 to fix the datetime test issue that is blocking CI along with a fix/workaround for test-benchmark-buffer which is blocking that PR from landing. Putting them together in one PR to get them both landed and hopefully get CI unstuck.

@Trott Trott added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 1, 2021
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. tools Issues and PRs related to the tools directory. labels Nov 1, 2021
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 1, 2021
@nodejs-github-bot

This comment has been minimized.

@targos
Copy link
Member

targos commented Nov 1, 2021

I think someone from build should still investigate why that windows host suddenly became slow

@Trott Trott added the fast-track PRs that do not need to wait for 48 hours to land. label Nov 1, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Nov 1, 2021

Fast-track has been requested by @Trott. Please 👍 to approve.

@Trott
Copy link
Member Author

Trott commented Nov 1, 2021

I think someone from build should still investigate why that windows host suddenly became slow

It's an Azure host so maybe someone at Microsoft can help? Hate to ping because I know they all have other things going on, but uh... @joaocgreis @bnb @bzoz This would be the win10-vs2019-COMPILED_BY-vs2019 jobs.

@nodejs-github-bot

This comment has been minimized.

@Trott
Copy link
Member Author

Trott commented Nov 1, 2021

It's an Azure host so maybe someone at Microsoft can help? Hate to ping because I know they all have other things going on, but uh... @joaocgreis @bnb @bzoz This would be the win10-vs2019-COMPILED_BY-vs2019 jobs.

A good example to look at might be https://ci.nodejs.org/job/node-test-binary-windows-native-suites/10883.

@Trott
Copy link
Member Author

Trott commented Nov 1, 2021

Argh, the test-policy-integrity pummel test is likewise timing out on a win10-COMPILED_BY-vs2019 machine. I'll bump up the pummel timeouts to 6x (from 4x).

@nodejs-github-bot

This comment has been minimized.

@targos
Copy link
Member

targos commented Nov 1, 2021

@Trott did you mean to push before rerunning CI?

@Trott
Copy link
Member Author

Trott commented Nov 1, 2021

@Trott did you mean to push before rerunning CI?

🤦 It's late here....

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@targos
Copy link
Member

targos commented Nov 1, 2021

Well, now the job is killed by Jenkins because it takes more than 10 minutes:

11:54:58 ok 791 pummel/test-net-timeout
11:54:58   ---
11:54:58   duration_ms: 5.233
11:54:58   ...
12:04:58 Build timed out (after 10 minutes). Marking the build as failed.
12:04:59 Terminate batch job (Y/N)? 

@Trott
Copy link
Member Author

Trott commented Nov 1, 2021

Well, now the job is killed by Jenkins because it takes more than 10 minutes:

11:54:58 ok 791 pummel/test-net-timeout
11:54:58   ---
11:54:58   duration_ms: 5.233
11:54:58   ...
12:04:58 Build timed out (after 10 minutes). Marking the build as failed.
12:04:59 Terminate batch job (Y/N)? 

I've increased the timeout for node-test-binary-windows-js-suites on Jenkins to 15 minutes (900 seconds up from 600 seconds). @nodejs/build Is there somewhere I should document this? (I'll open an issue in the build repo at least.)

@nodejs-github-bot

This comment has been minimized.

@Trott
Copy link
Member Author

Trott commented Nov 1, 2021

@tniessen (or another Collaborator): This could use a second 👍 on the fast track request comment at #40684 (comment).

@tniessen
Copy link
Member

tniessen commented Nov 1, 2021

@Trott 👍

@Trott
Copy link
Member Author

Trott commented Nov 1, 2021

I've increased the timeout for node-test-binary-windows-js-suites on Jenkins to 15 minutes (900 seconds up from 600 seconds).

Of course, if the problem is that there's a race condition and pummel/test-policy-integrity will never finish, then that won't help....

If that's the case, I'll insert some skip-if-on-Windows code into the test and open an issue for it. I hate to do that, but it is an experimental feature at least.

@Trott
Copy link
Member Author

Trott commented Nov 1, 2021

test-policy-integrity still failed on the Windows 10/VS2019 host after 12 minutes so I'm marking it flaky, which is unfortunate because it seems to fail approximately 100% of the time. But it's an experimental feature and we need to be able to land working code.

@nodejs-github-bot

This comment has been minimized.

@Trott
Copy link
Member Author

Trott commented Nov 1, 2021

image

@Trott
Copy link
Member Author

Trott commented Nov 1, 2021

I wonder if someone did something to fix the slow-running issue. test-policy-integrity timed out (more than 12 minutes) in https://ci.nodejs.org/job/node-test-pull-request/40619/ but took less than a minute to run on https://ci.nodejs.org/job/node-test-pull-request/40620/.

@Trott
Copy link
Member Author

Trott commented Nov 1, 2021

I wonder if someone did something to fix the slow-running issue. test-policy-integrity timed out (more than 12 minutes) in https://ci.nodejs.org/job/node-test-pull-request/40619/ but took less than a minute to run on https://ci.nodejs.org/job/node-test-pull-request/40620/.

(Regardless, let's land this, and we can revert or undo the greater time allowances if the fix is across everything and robust. It's not harming anything.)

Trott pushed a commit that referenced this pull request Nov 1, 2021
Add standard timezone name for Dublin without daylight saving

PR-URL: #40684
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Voltrex <mohammadkeyvanzade94@gmail.com>
Trott added a commit that referenced this pull request Nov 1, 2021
test-benchmark-buffer is consistently timing out on a single Windows
host in CI. Rather than try to figure out if we need to scale the
timeout up for a certain memory limit or chip speed or something else,
let's increase the timeout for benchmark tests in general.

PR-URL: #40684
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Voltrex <mohammadkeyvanzade94@gmail.com>
Trott added a commit that referenced this pull request Nov 1, 2021
The win10-COMPILED_BY-vs2019 hosts in CI are very slow and timing out on
certain tests in pummel and (previously) benchmark. Increase timeout
from 4x to 6x.

PR-URL: #40684
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Voltrex <mohammadkeyvanzade94@gmail.com>
@Trott Trott closed this Nov 1, 2021
@Trott
Copy link
Member Author

Trott commented Nov 1, 2021

Landed in 86099a3...b2aff85

@Trott Trott merged commit b2aff85 into nodejs:master Nov 1, 2021
@Trott Trott deleted the big-fix branch November 1, 2021 16:49
@Mesteery Mesteery added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 1, 2021
targos pushed a commit that referenced this pull request Nov 4, 2021
Add standard timezone name for Dublin without daylight saving

PR-URL: #40684
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Voltrex <mohammadkeyvanzade94@gmail.com>
targos pushed a commit that referenced this pull request Nov 4, 2021
test-benchmark-buffer is consistently timing out on a single Windows
host in CI. Rather than try to figure out if we need to scale the
timeout up for a certain memory limit or chip speed or something else,
let's increase the timeout for benchmark tests in general.

PR-URL: #40684
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Voltrex <mohammadkeyvanzade94@gmail.com>
targos pushed a commit that referenced this pull request Nov 4, 2021
The win10-COMPILED_BY-vs2019 hosts in CI are very slow and timing out on
certain tests in pummel and (previously) benchmark. Increase timeout
from 4x to 6x.

PR-URL: #40684
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Voltrex <mohammadkeyvanzade94@gmail.com>
targos pushed a commit that referenced this pull request Nov 4, 2021
Ref: #40694

PR-URL: #40684
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Voltrex <mohammadkeyvanzade94@gmail.com>
targos pushed a commit that referenced this pull request Nov 6, 2021
Add standard timezone name for Dublin without daylight saving

PR-URL: #40684
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Voltrex <mohammadkeyvanzade94@gmail.com>
targos pushed a commit that referenced this pull request Nov 6, 2021
test-benchmark-buffer is consistently timing out on a single Windows
host in CI. Rather than try to figure out if we need to scale the
timeout up for a certain memory limit or chip speed or something else,
let's increase the timeout for benchmark tests in general.

PR-URL: #40684
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Voltrex <mohammadkeyvanzade94@gmail.com>
targos pushed a commit that referenced this pull request Nov 6, 2021
The win10-COMPILED_BY-vs2019 hosts in CI are very slow and timing out on
certain tests in pummel and (previously) benchmark. Increase timeout
from 4x to 6x.

PR-URL: #40684
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Voltrex <mohammadkeyvanzade94@gmail.com>
targos pushed a commit that referenced this pull request Nov 6, 2021
Ref: #40694

PR-URL: #40684
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Voltrex <mohammadkeyvanzade94@gmail.com>
@targos targos mentioned this pull request Nov 8, 2021
@BethGriggs BethGriggs mentioned this pull request Nov 26, 2021
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. fast-track PRs that do not need to wait for 48 hours to land. needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants