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

ci: Re-enable and fix NSE test #3417

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

andybalaam
Copy link
Contributor

@andybalaam andybalaam commented May 15, 2024

Makes #3411 less bad because it should fail fast.

It also seems to make it fail less often, so it might be worth re-enabling the test (which this PR does) and seeing whether it gives us clues.

Re-opening this PR as @bnjbvr suggested it might be worth re-enabling the test. I'm happy to leave it disabled, delete it, or re-enable it as people wish.

@andybalaam
Copy link
Contributor Author

I've had to stop working on this.

@andybalaam andybalaam closed this May 16, 2024
@andybalaam andybalaam reopened this May 16, 2024
@andybalaam andybalaam force-pushed the andybalaam/unignore-nse-test branch from b5987a1 to c7d07cd Compare May 16, 2024 15:28
@andybalaam andybalaam marked this pull request as ready for review May 16, 2024 15:28
@andybalaam andybalaam requested a review from a team as a code owner May 16, 2024 15:28
@andybalaam andybalaam requested review from bnjbvr and removed request for a team May 16, 2024 15:28
Copy link

codecov bot commented May 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.19%. Comparing base (b57dbd8) to head (c7d07cd).
Report is 22 commits behind head on main.

Current head c7d07cd differs from pull request most recent head 6a2419f

Please upload reports for the commit 6a2419f to get more accurate results.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3417      +/-   ##
==========================================
+ Coverage   82.97%   83.19%   +0.22%     
==========================================
  Files         246      246              
  Lines       24958    24958              
==========================================
+ Hits        20708    20765      +57     
+ Misses       4250     4193      -57     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bnjbvr bnjbvr changed the title Re-enable and fix NSE test ci: Re-enable and fix NSE test May 17, 2024
@bnjbvr
Copy link
Member

bnjbvr commented May 17, 2024

Hah, managed to make it permafail in the CodeCov task just by rejiggering the logic.

I propose to shelve the PR, but keep it open; as soon as anyone has a bit of free time, they should feel free to investigate it, because it does seem to exhibit some real issues.

@bnjbvr bnjbvr removed their request for review May 17, 2024 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants