-
Notifications
You must be signed in to change notification settings - Fork 19
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 route delete bug #2273
Fix route delete bug #2273
Conversation
williamlardier
commented
Nov 29, 2024
•
edited
Loading
edited
Hello williamlardier,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
@@ -60,7 +60,7 @@ export default function routeDELETE( | |||
*/ | |||
if (err && ( | |||
!(err instanceof ArsenalError) || | |||
(!err.is.NoSuchKey && err.is.NoSuchVersion) | |||
(!err.is.NoSuchKey && !err.is.NoSuchVersion) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add unit tests for these functions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added some but they don't cover all the paths, I will add it (while I am confirming in cloudserver the fix)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(we had 0 unit test for the routes before)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 24a87f5
we already have it but it was not testing both cases...
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## development/8.1 #2273 +/- ##
================================================
Coverage 66.27% 66.27%
================================================
Files 215 215
Lines 17329 17329
Branches 3553 3529 -24
================================================
Hits 11485 11485
Misses 5829 5829
Partials 15 15 ☔ View full report in Codecov by Sentry. |
Request integration branchesWaiting for integration branch creation to be requested by the user. To request integration branches, please comment on this pull request with the following command:
Alternatively, the |
0421e54
to
7402f09
Compare
/create_integration_branches |
Integration data createdI have created the integration data for the additional destination branches.
The following branches will NOT be impacted:
You can set option
The following options are set: create_integration_branches |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
The following options are set: create_integration_branches |
/approve |
I have successfully merged the changeset of this pull request
The following branches have NOT changed:
Please check the status of the associated issue ARSN-448. Goodbye williamlardier. The following options are set: approve, create_integration_branches |