You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "afzal442 (via GitHub)" <gi...@apache.org> on 2023/05/03 07:35:58 UTC
[GitHub] [apisix-ingress-controller] afzal442 opened a new pull request, #1824: Refactor the apisix route validation test function
afzal442 opened a new pull request, #1824:
URL: https://github.com/apache/apisix-ingress-controller/pull/1824
<!-- Please answer these questions before submitting a pull request -->
### Type of change:
<!-- Please delete options that are not relevant. -->
<!-- Select all the options from below that matches the type your PR best -->
- [ ] Bugfix
- [ ] New feature provided
- [ ] Improve performance
- [ ] Backport patches
- [ ] Documentation
- [ ] Refactor
- [ ] Chore
- [ ] CI/CD or Tests
### What this PR does / why we need it:
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->
### Pre-submission checklist:
<!--
Please follow the requirements:
1. Use Draft if the PR is not ready to be reviewed
2. Test is required for the feat/fix PR, unless you have a good reason
3. Doc is required for the feat PR
4. Use a new commit to resolve review instead of `push -f`
5. Use "request review" to notify the reviewer once you have resolved the review
-->
- [ ] Did you explain what problem does this PR solve? Or what new features have been added?
- [ ] Have you added corresponding test cases?
- [ ] Have you modified the corresponding document?
- [ ] Is this PR backward compatible? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix-ingress-controller#community) first**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refactor the apisix route validation test function [apisix-ingress-controller]
Posted by "Revolyssup (via GitHub)" <gi...@apache.org>.
Revolyssup commented on PR #1824:
URL: https://github.com/apache/apisix-ingress-controller/pull/1824#issuecomment-3153815290
Since no reviewer had approved of this and ingress controller has been refactored as well. I am going to close this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] afzal442 commented on pull request #1824: Refactor the apisix route validation test function
Posted by "afzal442 (via GitHub)" <gi...@apache.org>.
afzal442 commented on PR #1824:
URL: https://github.com/apache/apisix-ingress-controller/pull/1824#issuecomment-1613577638
> The new test function name doesn't match the content.
Sorry I couldn't get it. but I think the new test function name matches as you can see its original file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] afzal442 commented on pull request #1824: Refactor the apisix route validation test function
Posted by "afzal442 (via GitHub)" <gi...@apache.org>.
afzal442 commented on PR #1824:
URL: https://github.com/apache/apisix-ingress-controller/pull/1824#issuecomment-1539593177
Sorry @tao12345666333 ! are those tests failing related to this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] afzal442 commented on pull request #1824: Refactor the apisix route validation test function
Posted by "afzal442 (via GitHub)" <gi...@apache.org>.
afzal442 commented on PR #1824:
URL: https://github.com/apache/apisix-ingress-controller/pull/1824#issuecomment-1532581907
After testing it's got
```
=== RUN TestValidateApisixRouteV2/validating_is_failed_due_to_unknown_plugin_name
2023-05-03T07:25:52Z error validation/utils.go:105 failed to get the schema of plugin Not-A-Plugin: can't find the plugin schema
--- PASS: TestValidateApisixRouteV2 (0.00s)
--- PASS: TestValidateApisixRouteV2/validating_is_successes (0.00s)
--- PASS: TestValidateApisixRouteV2/validating_is_failed_due_to_missing_required_fields (0.00s)
--- PASS: TestValidateApisixRouteV2/validating_is_failed_due_to_invalid_break_response_code (0.00s)
--- PASS: TestValidateApisixRouteV2/validating_is_failed_due_to_invalid_max_breaker_sec (0.00s)
--- PASS: TestValidateApisixRouteV2/validating_is_failed_due_to_unknown_plugin_name (0.00s)
--- PASS: TestValidateApisixRouteHTTPV2 (0.00s)
--- PASS: TestValidateApisixRouteHTTPV2/validating_is_successes (0.00s)
--- PASS: TestValidateApisixRouteHTTPV2/validating_is_failed_due_to_missing_required_fields (0.00s)
--- PASS: TestValidateApisixRouteHTTPV2/validating_is_failed_due_to_invalid_break_response_code (0.00s)
--- PASS: TestValidateApisixRouteHTTPV2/validating_is_failed_due_to_invalid_max_breaker_sec (0.00s)
--- PASS: TestValidateApisixRouteHTTPV2/validating_is_failed_due_to_unknown_plugin_name (0.00s)
```
cc @tao12345666333
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] afzalbin64 commented on pull request #1824: Refactor the apisix route validation test function
Posted by "afzalbin64 (via GitHub)" <gi...@apache.org>.
afzalbin64 commented on PR #1824:
URL: https://github.com/apache/apisix-ingress-controller/pull/1824#issuecomment-1577892222
@tao12345666333 can you review it as conflicts resolved?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] tao12345666333 commented on pull request #1824: Refactor the apisix route validation test function
Posted by "tao12345666333 (via GitHub)" <gi...@apache.org>.
tao12345666333 commented on PR #1824:
URL: https://github.com/apache/apisix-ingress-controller/pull/1824#issuecomment-1549166060
Sorry for the delay. Could you please resolve the conflicts?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refactor the apisix route validation test function [apisix-ingress-controller]
Posted by "afzal442 (via GitHub)" <gi...@apache.org>.
afzal442 commented on PR #1824:
URL: https://github.com/apache/apisix-ingress-controller/pull/1824#issuecomment-1903408914
@zll600 I am not able to understand you. what is the issue with this function `TestValidateApisixRouteV2(t *testing.T)` ?
> The test function did not use global variables.
What do yoi mean? In the future if you need to add or rmv test cases, what will yoiu do?
This latest one is much easier to read than original one. Let me know. Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] lingsamuel commented on pull request #1824: Refactor the apisix route validation test function
Posted by "lingsamuel (via GitHub)" <gi...@apache.org>.
lingsamuel commented on PR #1824:
URL: https://github.com/apache/apisix-ingress-controller/pull/1824#issuecomment-1612385537
I don't see any need for this PR. The new test function name doesn't match the content.
Can you explain why we need such change?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refactor the apisix route validation test function [apisix-ingress-controller]
Posted by "zll600 (via GitHub)" <gi...@apache.org>.
zll600 commented on PR #1824:
URL: https://github.com/apache/apisix-ingress-controller/pull/1824#issuecomment-1902825015
> > The new test function name doesn't match the content.
>
> Sorry I couldn't get it. but I think the new test function name matches as you can see its original file.
Could you explain in detail why this PR is needed? I think the original one is good enough. Because
- The test function did not use global variables.
- The name of test function matches the its original file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-ingress-controller] afzal442 commented on pull request #1824: Refactor the apisix route validation test function
Posted by "afzal442 (via GitHub)" <gi...@apache.org>.
afzal442 commented on PR #1824:
URL: https://github.com/apache/apisix-ingress-controller/pull/1824#issuecomment-1537844128
hi @tao12345666333 as a reminder, if you get time, you can review it. TY 🧷
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refactor the apisix route validation test function [apisix-ingress-controller]
Posted by "Revolyssup (via GitHub)" <gi...@apache.org>.
Revolyssup closed pull request #1824: Refactor the apisix route validation test function
URL: https://github.com/apache/apisix-ingress-controller/pull/1824
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org