You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "khareyash05 (via GitHub)" <gi...@apache.org> on 2023/04/29 18:19:33 UTC
[GitHub] [apisix-ingress-controller] khareyash05 opened a new pull request, #1822: Create apisix_plugin_config_test.go
khareyash05 opened a new pull request, #1822:
URL: https://github.com/apache/apisix-ingress-controller/pull/1822
<!-- 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:
Increase unit tests would enhance the code coverage
<!--- 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
-->
- [X] Did you explain what problem does this PR solve? Or what new features have been added?
- [X] Have you added corresponding test cases?
- [X] Have you modified the corresponding document?
- [X] 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
[GitHub] [apisix-ingress-controller] Gallardot commented on a diff in pull request #1822: Create apisix_plugin_config_test.go
Posted by "Gallardot (via GitHub)" <gi...@apache.org>.
Gallardot commented on code in PR #1822:
URL: https://github.com/apache/apisix-ingress-controller/pull/1822#discussion_r1181718453
##########
pkg/api/validation/apisix_plugin_config_test.go:
##########
@@ -0,0 +1,92 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package validation
+
+import (
+ "testing"
+
+ v2 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v2"
+)
+
+func TestValidateApisixRoutePlugins(t *testing.T) {
+ validPlugins := []v2.ApisixRoutePlugin{
+ {
+ Name: "plugin1",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value1",
+ "param2": 123,
+ },
+ },
+ {
+ Name: "plugin2",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value2",
+ },
+ },
+ }
+
+ invalidPlugins := []v2.ApisixRoutePlugin{
+ {
+ Name: "",
+ Config: v2.ApisixRoutePluginConfig{},
+ },
+ {
+ Name: "plugin1",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value1",
+ "param2": "invalid",
+ },
+ },
+ }
+
+ tests := []struct {
+ name string
+ plugins []v2.ApisixRoutePlugin
+ expectValid bool
+ expectError bool
+ }{
+ {
+ name: "Valid ApisixRoutePlugin objects",
+ plugins: validPlugins,
+ expectValid: true,
+ expectError: false,
+ },
+ {
+ name: "Invalid ApisixRoutePlugin objects with empty name and config",
+ plugins: []v2.ApisixRoutePlugin{
+ {Name: "", Config: v2.ApisixRoutePluginConfig{}},
+ },
+ expectValid: false,
+ expectError: false,
+ },
+ {
+ name: "Invalid ApisixRoutePlugin objects with invalid config value",
+ plugins: invalidPlugins,
+ expectValid: false,
+ expectError: false,
+ },
+ }
+
+ for _, tc := range tests {
+ t.Run(tc.name, func(t *testing.T) {
+ _, err := ValidateApisixRoutePlugins(tc.plugins)
Review Comment:
You should verify that the return value matches the expectations of the test case
--
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] Gallardot commented on pull request #1822: Create apisix_plugin_config_test.go
Posted by "Gallardot (via GitHub)" <gi...@apache.org>.
Gallardot commented on PR #1822:
URL: https://github.com/apache/apisix-ingress-controller/pull/1822#issuecomment-1534317028
I find that the current goal of this PR to test is actually the same as the goal of [Test_validatePlugin](https://github.com/apache/apisix-ingress-controller/blob/2182a48cbca785373eca745a13d8cf2b7d9ab6c8/pkg/api/validation/apisix_route_test.go#L72) to test, but there are some differences in the test cases. Would it be nice to add some more test cases on top of [Test_validatePlugin](https://github.com/apache/apisix-ingress-controller/blob/2182a48cbca785373eca745a13d8cf2b7d9ab6c8/pkg/api/validation/apisix_route_test.go#L72) ?
@khareyash05 @afzal442
--
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 a diff in pull request #1822: Create apisix_plugin_config_test.go
Posted by "afzal442 (via GitHub)" <gi...@apache.org>.
afzal442 commented on code in PR #1822:
URL: https://github.com/apache/apisix-ingress-controller/pull/1822#discussion_r1182074235
##########
pkg/api/validation/apisix_plugin_config_test.go:
##########
@@ -0,0 +1,92 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package validation
+
+import (
+ "testing"
+
+ v2 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v2"
+)
+
+func TestValidateApisixRoutePlugins(t *testing.T) {
+ validPlugins := []v2.ApisixRoutePlugin{
+ {
+ Name: "plugin1",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value1",
+ "param2": 123,
+ },
+ },
+ {
+ Name: "plugin2",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value2",
+ },
+ },
+ }
+
+ invalidPlugins := []v2.ApisixRoutePlugin{
+ {
+ Name: "",
+ Config: v2.ApisixRoutePluginConfig{},
+ },
+ {
+ Name: "plugin1",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value1",
+ "param2": "invalid",
+ },
+ },
+ }
+
+ tests := []struct {
+ name string
+ plugins []v2.ApisixRoutePlugin
+ expectValid bool
+ expectError bool
+ }{
+ {
+ name: "Valid ApisixRoutePlugin objects",
+ plugins: validPlugins,
+ expectValid: true,
+ expectError: false,
+ },
+ {
+ name: "Invalid ApisixRoutePlugin objects with empty name and config",
+ plugins: []v2.ApisixRoutePlugin{
+ {Name: "", Config: v2.ApisixRoutePluginConfig{}},
+ },
+ expectValid: false,
+ expectError: false,
+ },
+ {
+ name: "Invalid ApisixRoutePlugin objects with invalid config value",
+ plugins: invalidPlugins,
+ expectValid: false,
+ expectError: false,
+ },
+ }
+
+ for _, tc := range tests {
+ t.Run(tc.name, func(t *testing.T) {
+ _, err := ValidateApisixRoutePlugins(tc.plugins)
Review Comment:
@khareyash05 you have to call `ValidateApisixPluginConfigV2` to verify as told by @Gallardot . You can do like below and compare it with your `expectValid` and ignore error.
```suggestion
gotValid, _ := ValidateApisixRoutePlugins(tc.plugins)
if gotValid != tc.expectValid {
t.Errorf("ValidateApisixRoutePlugins() gotValid = %v, expect %v", gotValid, tc.ExpectValid)
}
```
Also you can take a look at the ApisixPluginConfig definition and restructure your specs of it https://github.com/apache/apisix-ingress-controller/blob/2182a48cbca785373eca745a13d8cf2b7d9ab6c8/pkg/kube/apisix/apis/config/v2/types.go#L796.
https://github.com/apache/apisix-ingress-controller/blob/2182a48cbca785373eca745a13d8cf2b7d9ab6c8/pkg/kube/apisix/apis/config/v2/types.go#L786
--
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] khareyash05 commented on a diff in pull request #1822: Create apisix_plugin_config_test.go
Posted by "khareyash05 (via GitHub)" <gi...@apache.org>.
khareyash05 commented on code in PR #1822:
URL: https://github.com/apache/apisix-ingress-controller/pull/1822#discussion_r1182693318
##########
pkg/api/validation/apisix_plugin_config_test.go:
##########
@@ -0,0 +1,94 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package validation
+
+import (
+ "testing"
+
+ v2 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v2"
+)
+
+func TestValidateApisixRoutePlugins(t *testing.T) {
+ validPlugins := []v2.ApisixRoutePlugin{
+ {
+ Name: "plugin1",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value1",
+ "param2": 123,
+ },
+ },
+ {
+ Name: "plugin2",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value2",
+ },
+ },
+ }
+
+ invalidPlugins := []v2.ApisixRoutePlugin{
+ {
+ Name: "",
+ Config: v2.ApisixRoutePluginConfig{},
+ },
+ {
+ Name: "plugin1",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value1",
+ "param2": "invalid",
+ },
+ },
+ }
+
+ tests := []struct {
+ name string
+ plugins []v2.ApisixRoutePlugin
+ expectValid bool
+ expectError bool
+ }{
+ {
+ name: "Valid ApisixRoutePlugin objects",
+ plugins: validPlugins,
+ expectValid: true,
+ expectError: false,
+ },
+ {
+ name: "Invalid ApisixRoutePlugin objects with empty name and config",
+ plugins: []v2.ApisixRoutePlugin{
+ {Name: "", Config: v2.ApisixRoutePluginConfig{}},
+ },
+ expectValid: false,
+ expectError: false,
+ },
+ {
+ name: "Invalid ApisixRoutePlugin objects with invalid config value",
+ plugins: invalidPlugins,
+ expectValid: false,
+ expectError: false,
+ },
+ }
+
+ for _, tc := range tests {
+ t.Run(tc.name, func(t *testing.T) {
+ gotValid, err := ValidateApisixRoutePlugins(tc.plugins)
Review Comment:
so instead of `ValidateApisixRoutePlugins` should I call `ValidatePlugin`?
--
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] khareyash05 commented on a diff in pull request #1822: Create apisix_plugin_config_test.go
Posted by "khareyash05 (via GitHub)" <gi...@apache.org>.
khareyash05 commented on code in PR #1822:
URL: https://github.com/apache/apisix-ingress-controller/pull/1822#discussion_r1181738802
##########
pkg/api/validation/apisix_plugin_config_test.go:
##########
@@ -0,0 +1,92 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package validation
+
+import (
+ "testing"
+
+ v2 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v2"
+)
+
+func TestValidateApisixRoutePlugins(t *testing.T) {
+ validPlugins := []v2.ApisixRoutePlugin{
+ {
+ Name: "plugin1",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value1",
+ "param2": 123,
+ },
+ },
+ {
+ Name: "plugin2",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value2",
+ },
+ },
+ }
+
+ invalidPlugins := []v2.ApisixRoutePlugin{
+ {
+ Name: "",
+ Config: v2.ApisixRoutePluginConfig{},
+ },
+ {
+ Name: "plugin1",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value1",
+ "param2": "invalid",
+ },
+ },
+ }
+
+ tests := []struct {
+ name string
+ plugins []v2.ApisixRoutePlugin
+ expectValid bool
+ expectError bool
+ }{
+ {
+ name: "Valid ApisixRoutePlugin objects",
+ plugins: validPlugins,
+ expectValid: true,
+ expectError: false,
+ },
+ {
+ name: "Invalid ApisixRoutePlugin objects with empty name and config",
+ plugins: []v2.ApisixRoutePlugin{
+ {Name: "", Config: v2.ApisixRoutePluginConfig{}},
+ },
+ expectValid: false,
+ expectError: false,
+ },
+ {
+ name: "Invalid ApisixRoutePlugin objects with invalid config value",
+ plugins: invalidPlugins,
+ expectValid: false,
+ expectError: false,
+ },
+ }
+
+ for _, tc := range tests {
+ t.Run(tc.name, func(t *testing.T) {
+ _, err := ValidateApisixRoutePlugins(tc.plugins)
Review Comment:
Only the data type to be checked right?
--
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] khareyash05 commented on a diff in pull request #1822: Create apisix_plugin_config_test.go
Posted by "khareyash05 (via GitHub)" <gi...@apache.org>.
khareyash05 commented on code in PR #1822:
URL: https://github.com/apache/apisix-ingress-controller/pull/1822#discussion_r1182696599
##########
pkg/api/validation/apisix_plugin_config_test.go:
##########
@@ -0,0 +1,94 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package validation
+
+import (
+ "testing"
+
+ v2 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v2"
+)
+
+func TestValidateApisixRoutePlugins(t *testing.T) {
+ validPlugins := []v2.ApisixRoutePlugin{
+ {
+ Name: "plugin1",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value1",
+ "param2": 123,
+ },
+ },
+ {
+ Name: "plugin2",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value2",
+ },
+ },
+ }
+
+ invalidPlugins := []v2.ApisixRoutePlugin{
+ {
+ Name: "",
+ Config: v2.ApisixRoutePluginConfig{},
+ },
+ {
+ Name: "plugin1",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value1",
+ "param2": "invalid",
+ },
+ },
+ }
+
+ tests := []struct {
+ name string
+ plugins []v2.ApisixRoutePlugin
+ expectValid bool
+ expectError bool
+ }{
+ {
+ name: "Valid ApisixRoutePlugin objects",
+ plugins: validPlugins,
+ expectValid: true,
+ expectError: false,
+ },
+ {
+ name: "Invalid ApisixRoutePlugin objects with empty name and config",
+ plugins: []v2.ApisixRoutePlugin{
+ {Name: "", Config: v2.ApisixRoutePluginConfig{}},
+ },
+ expectValid: false,
+ expectError: false,
+ },
+ {
+ name: "Invalid ApisixRoutePlugin objects with invalid config value",
+ plugins: invalidPlugins,
+ expectValid: false,
+ expectError: false,
+ },
+ }
+
+ for _, tc := range tests {
+ t.Run(tc.name, func(t *testing.T) {
+ gotValid, err := ValidateApisixRoutePlugins(tc.plugins)
Review Comment:
I am not able to get an idea....should I add the `ValidatePlugin` before `ValidateApisixRoutePlugins`? because `ValidateApisixRoutePlugins` takes only plugins as a parameter... I am a bit confused ..sorry
--
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] github-actions[bot] closed pull request #1822: Create apisix_plugin_config_test.go
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #1822: Create apisix_plugin_config_test.go
URL: https://github.com/apache/apisix-ingress-controller/pull/1822
--
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] khareyash05 commented on a diff in pull request #1822: Create apisix_plugin_config_test.go
Posted by "khareyash05 (via GitHub)" <gi...@apache.org>.
khareyash05 commented on code in PR #1822:
URL: https://github.com/apache/apisix-ingress-controller/pull/1822#discussion_r1181720515
##########
pkg/api/validation/apisix_plugin_config_test.go:
##########
@@ -0,0 +1,92 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package validation
+
+import (
+ "testing"
+
+ v2 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v2"
+)
+
+func TestValidateApisixRoutePlugins(t *testing.T) {
+ validPlugins := []v2.ApisixRoutePlugin{
+ {
+ Name: "plugin1",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value1",
+ "param2": 123,
+ },
+ },
+ {
+ Name: "plugin2",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value2",
+ },
+ },
+ }
+
+ invalidPlugins := []v2.ApisixRoutePlugin{
+ {
+ Name: "",
+ Config: v2.ApisixRoutePluginConfig{},
+ },
+ {
+ Name: "plugin1",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value1",
+ "param2": "invalid",
+ },
+ },
+ }
+
+ tests := []struct {
+ name string
+ plugins []v2.ApisixRoutePlugin
+ expectValid bool
+ expectError bool
+ }{
+ {
+ name: "Valid ApisixRoutePlugin objects",
+ plugins: validPlugins,
+ expectValid: true,
+ expectError: false,
+ },
+ {
+ name: "Invalid ApisixRoutePlugin objects with empty name and config",
+ plugins: []v2.ApisixRoutePlugin{
+ {Name: "", Config: v2.ApisixRoutePluginConfig{}},
+ },
+ expectValid: false,
+ expectError: false,
+ },
+ {
+ name: "Invalid ApisixRoutePlugin objects with invalid config value",
+ plugins: invalidPlugins,
+ expectValid: false,
+ expectError: false,
+ },
+ }
+
+ for _, tc := range tests {
+ t.Run(tc.name, func(t *testing.T) {
+ _, err := ValidateApisixRoutePlugins(tc.plugins)
Review Comment:
you mean i should check the value _ returns as well . Am i right?
--
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] khareyash05 commented on a diff in pull request #1822: Create apisix_plugin_config_test.go
Posted by "khareyash05 (via GitHub)" <gi...@apache.org>.
khareyash05 commented on code in PR #1822:
URL: https://github.com/apache/apisix-ingress-controller/pull/1822#discussion_r1182697856
##########
pkg/api/validation/apisix_plugin_config_test.go:
##########
@@ -0,0 +1,94 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package validation
+
+import (
+ "testing"
+
+ v2 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v2"
+)
+
+func TestValidateApisixRoutePlugins(t *testing.T) {
+ validPlugins := []v2.ApisixRoutePlugin{
+ {
+ Name: "plugin1",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value1",
+ "param2": 123,
+ },
+ },
+ {
+ Name: "plugin2",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value2",
+ },
+ },
+ }
+
+ invalidPlugins := []v2.ApisixRoutePlugin{
+ {
+ Name: "",
+ Config: v2.ApisixRoutePluginConfig{},
+ },
+ {
+ Name: "plugin1",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value1",
+ "param2": "invalid",
+ },
+ },
+ }
+
+ tests := []struct {
Review Comment:
If I add `ValidatePlugin` then will have to update the test cases. @Gallardot Kindly review.
--
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] Gallardot commented on a diff in pull request #1822: Create apisix_plugin_config_test.go
Posted by "Gallardot (via GitHub)" <gi...@apache.org>.
Gallardot commented on code in PR #1822:
URL: https://github.com/apache/apisix-ingress-controller/pull/1822#discussion_r1181724643
##########
pkg/api/validation/apisix_plugin_config_test.go:
##########
@@ -0,0 +1,92 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package validation
+
+import (
+ "testing"
+
+ v2 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v2"
+)
+
+func TestValidateApisixRoutePlugins(t *testing.T) {
+ validPlugins := []v2.ApisixRoutePlugin{
+ {
+ Name: "plugin1",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value1",
+ "param2": 123,
+ },
+ },
+ {
+ Name: "plugin2",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value2",
+ },
+ },
+ }
+
+ invalidPlugins := []v2.ApisixRoutePlugin{
+ {
+ Name: "",
+ Config: v2.ApisixRoutePluginConfig{},
+ },
+ {
+ Name: "plugin1",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value1",
+ "param2": "invalid",
+ },
+ },
+ }
+
+ tests := []struct {
+ name string
+ plugins []v2.ApisixRoutePlugin
+ expectValid bool
+ expectError bool
+ }{
+ {
+ name: "Valid ApisixRoutePlugin objects",
+ plugins: validPlugins,
+ expectValid: true,
+ expectError: false,
+ },
+ {
+ name: "Invalid ApisixRoutePlugin objects with empty name and config",
+ plugins: []v2.ApisixRoutePlugin{
+ {Name: "", Config: v2.ApisixRoutePluginConfig{}},
+ },
+ expectValid: false,
+ expectError: false,
+ },
+ {
+ name: "Invalid ApisixRoutePlugin objects with invalid config value",
+ plugins: invalidPlugins,
+ expectValid: false,
+ expectError: false,
+ },
+ }
+
+ for _, tc := range tests {
+ t.Run(tc.name, func(t *testing.T) {
+ _, err := ValidateApisixRoutePlugins(tc.plugins)
Review Comment:
https://github.com/apache/apisix-ingress-controller/blob/2182a48cbca785373eca745a13d8cf2b7d9ab6c8/pkg/api/validation/apisix_plugin_config.go#L25
Yes, according to the definition of this function, it should verify that `valid` and `resultErr` conform to the expectations of the test case
--
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 a diff in pull request #1822: Create apisix_plugin_config_test.go
Posted by "afzal442 (via GitHub)" <gi...@apache.org>.
afzal442 commented on code in PR #1822:
URL: https://github.com/apache/apisix-ingress-controller/pull/1822#discussion_r1182810923
##########
pkg/api/validation/apisix_plugin_config_test.go:
##########
@@ -0,0 +1,94 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package validation
+
+import (
+ "testing"
+
+ v2 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v2"
+)
+
+func TestValidateApisixRoutePlugins(t *testing.T) {
+ validPlugins := []v2.ApisixRoutePlugin{
+ {
+ Name: "plugin1",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value1",
+ "param2": 123,
+ },
+ },
+ {
+ Name: "plugin2",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value2",
+ },
+ },
+ }
+
+ invalidPlugins := []v2.ApisixRoutePlugin{
+ {
+ Name: "",
+ Config: v2.ApisixRoutePluginConfig{},
+ },
+ {
+ Name: "plugin1",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value1",
+ "param2": "invalid",
+ },
+ },
+ }
+
+ tests := []struct {
Review Comment:
Let's wait for it to get reviewed. 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
[GitHub] [apisix-ingress-controller] github-actions[bot] commented on pull request #1822: Create apisix_plugin_config_test.go
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #1822:
URL: https://github.com/apache/apisix-ingress-controller/pull/1822#issuecomment-1679830718
This pull request/issue has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
--
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] khareyash05 commented on pull request #1822: Create apisix_plugin_config_test.go
Posted by "khareyash05 (via GitHub)" <gi...@apache.org>.
khareyash05 commented on PR #1822:
URL: https://github.com/apache/apisix-ingress-controller/pull/1822#issuecomment-1528923342
@Gallardot have a look
--
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] khareyash05 commented on a diff in pull request #1822: Create apisix_plugin_config_test.go
Posted by "khareyash05 (via GitHub)" <gi...@apache.org>.
khareyash05 commented on code in PR #1822:
URL: https://github.com/apache/apisix-ingress-controller/pull/1822#discussion_r1181738802
##########
pkg/api/validation/apisix_plugin_config_test.go:
##########
@@ -0,0 +1,92 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package validation
+
+import (
+ "testing"
+
+ v2 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v2"
+)
+
+func TestValidateApisixRoutePlugins(t *testing.T) {
+ validPlugins := []v2.ApisixRoutePlugin{
+ {
+ Name: "plugin1",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value1",
+ "param2": 123,
+ },
+ },
+ {
+ Name: "plugin2",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value2",
+ },
+ },
+ }
+
+ invalidPlugins := []v2.ApisixRoutePlugin{
+ {
+ Name: "",
+ Config: v2.ApisixRoutePluginConfig{},
+ },
+ {
+ Name: "plugin1",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value1",
+ "param2": "invalid",
+ },
+ },
+ }
+
+ tests := []struct {
+ name string
+ plugins []v2.ApisixRoutePlugin
+ expectValid bool
+ expectError bool
+ }{
+ {
+ name: "Valid ApisixRoutePlugin objects",
+ plugins: validPlugins,
+ expectValid: true,
+ expectError: false,
+ },
+ {
+ name: "Invalid ApisixRoutePlugin objects with empty name and config",
+ plugins: []v2.ApisixRoutePlugin{
+ {Name: "", Config: v2.ApisixRoutePluginConfig{}},
+ },
+ expectValid: false,
+ expectError: false,
+ },
+ {
+ name: "Invalid ApisixRoutePlugin objects with invalid config value",
+ plugins: invalidPlugins,
+ expectValid: false,
+ expectError: false,
+ },
+ }
+
+ for _, tc := range tests {
+ t.Run(tc.name, func(t *testing.T) {
+ _, err := ValidateApisixRoutePlugins(tc.plugins)
Review Comment:
Only the data type to be checked right and if it exist or not?
--
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 a diff in pull request #1822: Create apisix_plugin_config_test.go
Posted by "afzal442 (via GitHub)" <gi...@apache.org>.
afzal442 commented on code in PR #1822:
URL: https://github.com/apache/apisix-ingress-controller/pull/1822#discussion_r1182074235
##########
pkg/api/validation/apisix_plugin_config_test.go:
##########
@@ -0,0 +1,92 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package validation
+
+import (
+ "testing"
+
+ v2 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v2"
+)
+
+func TestValidateApisixRoutePlugins(t *testing.T) {
+ validPlugins := []v2.ApisixRoutePlugin{
+ {
+ Name: "plugin1",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value1",
+ "param2": 123,
+ },
+ },
+ {
+ Name: "plugin2",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value2",
+ },
+ },
+ }
+
+ invalidPlugins := []v2.ApisixRoutePlugin{
+ {
+ Name: "",
+ Config: v2.ApisixRoutePluginConfig{},
+ },
+ {
+ Name: "plugin1",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value1",
+ "param2": "invalid",
+ },
+ },
+ }
+
+ tests := []struct {
+ name string
+ plugins []v2.ApisixRoutePlugin
+ expectValid bool
+ expectError bool
+ }{
+ {
+ name: "Valid ApisixRoutePlugin objects",
+ plugins: validPlugins,
+ expectValid: true,
+ expectError: false,
+ },
+ {
+ name: "Invalid ApisixRoutePlugin objects with empty name and config",
+ plugins: []v2.ApisixRoutePlugin{
+ {Name: "", Config: v2.ApisixRoutePluginConfig{}},
+ },
+ expectValid: false,
+ expectError: false,
+ },
+ {
+ name: "Invalid ApisixRoutePlugin objects with invalid config value",
+ plugins: invalidPlugins,
+ expectValid: false,
+ expectError: false,
+ },
+ }
+
+ for _, tc := range tests {
+ t.Run(tc.name, func(t *testing.T) {
+ _, err := ValidateApisixRoutePlugins(tc.plugins)
Review Comment:
@khareyash05 you have to call `ValidateApisixPluginConfigV2` to verify as told by @Gallardot . You can do like below and compare it with your `expectValid`
```suggestion
gotValid, err := ValidateApisixRoutePlugins(tc.plugins)
```
Also you can take a look at the ApisixPluginConfig definition and restructure your specs of it https://github.com/apache/apisix-ingress-controller/blob/2182a48cbca785373eca745a13d8cf2b7d9ab6c8/pkg/kube/apisix/apis/config/v2/types.go#L796.
https://github.com/apache/apisix-ingress-controller/blob/2182a48cbca785373eca745a13d8cf2b7d9ab6c8/pkg/kube/apisix/apis/config/v2/types.go#L786
--
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 a diff in pull request #1822: Create apisix_plugin_config_test.go
Posted by "afzal442 (via GitHub)" <gi...@apache.org>.
afzal442 commented on code in PR #1822:
URL: https://github.com/apache/apisix-ingress-controller/pull/1822#discussion_r1182656230
##########
pkg/api/validation/apisix_plugin_config_test.go:
##########
@@ -0,0 +1,94 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package validation
+
+import (
+ "testing"
+
+ v2 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v2"
+)
+
+func TestValidateApisixRoutePlugins(t *testing.T) {
+ validPlugins := []v2.ApisixRoutePlugin{
+ {
+ Name: "plugin1",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value1",
+ "param2": 123,
+ },
+ },
+ {
+ Name: "plugin2",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value2",
+ },
+ },
+ }
+
+ invalidPlugins := []v2.ApisixRoutePlugin{
+ {
+ Name: "",
+ Config: v2.ApisixRoutePluginConfig{},
+ },
+ {
+ Name: "plugin1",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value1",
+ "param2": "invalid",
+ },
+ },
+ }
+
+ tests := []struct {
Review Comment:
You may use the same test cases. I am not sure but @Gallardot can tell this more.
https://github.com/apache/apisix-ingress-controller/blob/2182a48cbca785373eca745a13d8cf2b7d9ab6c8/pkg/api/validation/apisix_route_test.go#L73-L76
##########
pkg/api/validation/apisix_plugin_config_test.go:
##########
@@ -0,0 +1,94 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package validation
+
+import (
+ "testing"
+
+ v2 "github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v2"
+)
+
+func TestValidateApisixRoutePlugins(t *testing.T) {
+ validPlugins := []v2.ApisixRoutePlugin{
+ {
+ Name: "plugin1",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value1",
+ "param2": 123,
+ },
+ },
+ {
+ Name: "plugin2",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value2",
+ },
+ },
+ }
+
+ invalidPlugins := []v2.ApisixRoutePlugin{
+ {
+ Name: "",
+ Config: v2.ApisixRoutePluginConfig{},
+ },
+ {
+ Name: "plugin1",
+ Config: v2.ApisixRoutePluginConfig{
+ "param1": "value1",
+ "param2": "invalid",
+ },
+ },
+ }
+
+ tests := []struct {
+ name string
+ plugins []v2.ApisixRoutePlugin
+ expectValid bool
+ expectError bool
+ }{
+ {
+ name: "Valid ApisixRoutePlugin objects",
+ plugins: validPlugins,
+ expectValid: true,
+ expectError: false,
+ },
+ {
+ name: "Invalid ApisixRoutePlugin objects with empty name and config",
+ plugins: []v2.ApisixRoutePlugin{
+ {Name: "", Config: v2.ApisixRoutePluginConfig{}},
+ },
+ expectValid: false,
+ expectError: false,
+ },
+ {
+ name: "Invalid ApisixRoutePlugin objects with invalid config value",
+ plugins: invalidPlugins,
+ expectValid: false,
+ expectError: false,
+ },
+ }
+
+ for _, tc := range tests {
+ t.Run(tc.name, func(t *testing.T) {
+ gotValid, err := ValidateApisixRoutePlugins(tc.plugins)
Review Comment:
You can follow this test file to create fake schema so you don't get error.
https://github.com/apache/apisix-ingress-controller/blob/2182a48cbca785373eca745a13d8cf2b7d9ab6c8/pkg/api/validation/apisix_route_test.go#L126
--
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] github-actions[bot] commented on pull request #1822: Create apisix_plugin_config_test.go
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #1822:
URL: https://github.com/apache/apisix-ingress-controller/pull/1822#issuecomment-1636941678
This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 30 days if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the dev@apisix.apache.org list. Thank you for your contributions.
--
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