-
Notifications
You must be signed in to change notification settings - Fork 40
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
fix: unbreak NewAPIErrorWithRaw and NewAPIError-related tests #267
Conversation
7781803
to
1ef0606
Compare
Codecov ReportBase: 52.84% // Head: 52.87% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #267 +/- ##
==========================================
+ Coverage 52.84% 52.87% +0.03%
==========================================
Files 51 51
Lines 4627 4647 +20
==========================================
+ Hits 2445 2457 +12
- Misses 1659 1665 +6
- Partials 523 525 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
I think I'd just try to address the failing test rather than doing that. Or perhaps change the API to accept options like |
f4fe5f2
to
d08501a
Compare
5c1a72f
to
156762a
Compare
156762a
to
0643c89
Compare
Yah, sorry, my bad, was overthinking it and mixed up the tests. |
In retrospect from #237, we actually do want to break the exported
NewAPIError
function. AFAICT we use this in one place only, and that place is a test within KIC that we can change trivially (searched through deck and don't see any use there).Introducing the separate one apparently broke all sorts of other things in go-kong's internal test expectations and apparently this repo is not configured to block auto-merge on that 😭
https://github.com/Kong/go-kong/actions/runs/3962834113/jobs/6790000801 suggests that the skipped
test-passed
actually satisfies the branch protection requirements, somehow.