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.

Sign up for GitHub

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

Jump to bottom

test: mark test-tls-min-max-version as flaky #56747

Closed
wants to merge 1 commit into from
Closed

test: mark test-tls-min-max-version as flaky #56747

wants to merge 1 commit into from

Conversation

JonasBa
Copy link
Contributor

@JonasBa JonasBa commented Jan 24, 2025

Ref: #56746

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Jan 24, 2025
@anonrig anonrig added smartos Issues and PRs related to the SmartOS platform. request-ci Add this label to start a Jenkins CI on a PR. labels Jan 24, 2025
@anonrig
Copy link
Member

anonrig commented Jan 24, 2025

cc @nodejs/platform-smartos

@anonrig anonrig requested a review from jasnell January 24, 2025 15:50
@anonrig anonrig added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 24, 2025
@JonasBa JonasBa mentioned this pull request Jan 24, 2025
Merged
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 24, 2025
@nodejs-github-bot
Copy link
Collaborator

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

@lpinca
Copy link
Member

lpinca commented Jan 24, 2025

The failure is a timeout so it might be #54918 again.

@anonrig
Copy link
Member

anonrig commented Jan 24, 2025

The failure is a timeout so it might be #54918 again.

@lpinca For education purposes: How did you know that it was a timeout? The error code is -15, which I couldn't find any information about online. Is it the duration that gave it away?

@lpinca
Copy link
Member

lpinca commented Jan 24, 2025 

duration_ms: 300088.347
exitcode: -15
severity: fail
stack: |-
  timeout
...

@anonrig anonrig requested a review from mcollina January 24, 2025 16:22
mcollina
mcollina approved these changes Jan 24, 2025
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

I think this is our deadlock problem :(.

@lpinca
Copy link
Member

lpinca commented Jan 24, 2025 •
edited
Loading

Is the test failing consistently or is it one off (timeout only)? In the latter case I would not mark it flaky or we will slowing mark every test flaky until a fix for #54918 is found.

@lpinca
Copy link
Member

lpinca commented Jan 24, 2025 •
edited
Loading

I could not find any previous failure of this test on https://github.com/nodejs/reliability so I think this is premature, but I don't want to start another fight :)

@codecov Codecov
Copy link

codecov bot commented Jan 24, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.21%. Comparing base (08eeddf) to head (1120f7f).
Report is 426 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main   #56747   +/-   ##
=======================================
  Coverage   89.21%   89.21%           
=======================================
  Files         662      662           
  Lines      191968   191968           
  Branches    36955    36948    -7     
=======================================
+ Hits       171269   171271    +2     
- Misses      13535    13542    +7     
+ Partials     7164     7155    -9

see 39 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

mcollina
mcollina requested changes Jan 24, 2025
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

blocking for now. Happy to revise if this show up further.

@aduh95 aduh95 removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 12, 2025
@JonasBa JonasBa closed this Mar 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcollina mcollina mcollina requested changes

@jasnell jasnell jasnell approved these changes

@anonrig anonrig anonrig approved these changes

Assignees
No one assigned
Labels
needs-ci PRs that need a full CI run. smartos Issues and PRs related to the SmartOS platform. 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
@JonasBa @anonrig @nodejs-github-bot @lpinca @mcollina @jasnell @aduh95