[GitHub] [apisix-dashboard] Jaycean commented on pull request #1245: feat(BE): Export route from OpenAPI Specification3.0
Jaycean commented on pull request #1245: URL: https://github.com/apache/apisix-dashboard/pull/1245#issuecomment-767936462 @starsz @nic-chen PTAL. Thks. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [apisix-dashboard] Jaycean commented on pull request #1245: feat(BE): Export route from OpenAPI Specification3.0
Jaycean commented on pull request #1245: URL: https://github.com/apache/apisix-dashboard/pull/1245#issuecomment-766539166 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [apisix-dashboard] Jaycean commented on pull request #1245: feat(BE): Export route from OpenAPI Specification3.0
Jaycean commented on pull request #1245: URL: https://github.com/apache/apisix-dashboard/pull/1245#issuecomment-766610459 In my opinion, from the perspective of apimix platform, I choose scheme a to satisfy the users of apimix platform. The function that URI can be renamed is more open and friendly for users. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [apisix-dashboard] Jaycean commented on pull request #1245: feat(BE): Export route from OpenAPI Specification3.0
Jaycean commented on pull request #1245:
URL: https://github.com/apache/apisix-dashboard/pull/1245#issuecomment-766539166
@liuxiran @nic-chen
The unit test and test cases have been supplemented. In the process of
testing, I found a problem：
- At present, the path data structure of OpenAPI 3 is used to store URIs,
and the data structure is map [string]. If the URI is repeated, the data will
be covered, if the repeated URIs are directly covered, only one URI can be
exported, which will lead to the loss of user data.
- My idea is to add a uniform suffix after the same URI when exporting, so
that the import can recognize the URI
For example：
```
{
"components": {},
"info": {
"title": "RoutesExport",
"version": "3.0.0"
},
"openapi": "3.0.0",
"paths": {
"/hello": {
"get": {}
}
"/hello-repeaturi2": {
"get": {}
}
"/hello-repeaturi2": {
"get": {}
}
}
}
```
- Do you have any better suggestions? We can discuss them. Thks.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [apisix-dashboard] Jaycean commented on pull request #1245: feat(BE): Export route from OpenAPI Specification3.0
Jaycean commented on pull request #1245:
URL: https://github.com/apache/apisix-dashboard/pull/1245#issuecomment-765474916
> > > > > > Hi, @Jaycean. Need unit test.
> > > > >
> > > > >
> > > > > At present, the function interface needs to export the existing
route data and ensure the correctness of the exported data, so I think E2E
should be written more perfectly. I think if the simple unit test only tests
whether the interface can export data, it does not represent whether the
function of the interface is correct, and if the relevant route data is not
created, the interface will not work normally use.
> > > >
> > > >
> > > > Not agree with you.
> > > > First of all, unit testing is very important. It helps find bugs
easily and quickly if someone changes the code.And unit testing can cover more
code than e2e test.
> > > > For example, in this case.
> > > > We need to test these situations at least:
> > > >
> > > > 1. route can't found
> > > > 2. Uris contain "*"
> > > > 3. plugins with `request-validation`
> > > > 4. different route method
> > > >...
> > > >
> > > > > and if the relevant route data is not created, the interface will
not work normally use.
> > > >
> > > >
> > > > We can mock data in etcd.
> > > > See
https://github.com/apache/apisix-dashboard/blob/master/api/internal/handler/server_info/server_info_test.go
for more details.
> > >
> > >
> > > @starsz Now I have encountered a problem in unit testing, exporting
the data that needs to create route, upstream and service. After reading the
example you gave me, I still don't know how to write it. Can you give me some
advice. Thks
>
> OK. Sorry, we don't have a doc to introduce the test contribute doc yet.
>
> First, you should know the mock technology.
> You can reference the doc:
https://github.com/stretchr/testify#mock-package.
> Mock provides a way to check input and give the return value that we want.
>
> In your code, we should mock those function:
>
> ```
> routeStore.Get
> routeStore.List
> upstreamStore.Get
> serviceStore.Get
> ```
>
> Second, IMO, you should write two testcases
> One test case for `ExportRoutes`, like:
> `TestDataLoader_ExportRoutes`
> And another test case for `ExportAllRoutes`.like
> `TestDataLoader_ExportAllRoutes`
>
> Third, in those test case function.We should mock the store like:
>
> ```
> routeMockStore := &store.MockInterface{}
> routeMockStore.On("Get", mock.Anything, mock.Anything).Run(func(args
mock.Arguments) {
> id := args.Get(0).(string)
> // here we can do some check on the id.
> }).Return(tc.giveRet, tc.giveErr)
> ```
>
> Mock.Anything stands for the args when you called routeMockStore.Get.
> tc.giveRet and tc.giveErr means what you want to get when called this line:
>
https://github.com/apache/apisix-dashboard/pull/1245/files#diff-195642dee38f4cdcc68921704b4b59dce5afc67d06074c1bc53004eb9b5dfc5eR70
> The same as other mock functions.
>
> Any feedback is welcome.
Thank you very much. I'll try to write it.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [apisix-dashboard] Jaycean commented on pull request #1245: feat(BE): Export route from OpenAPI Specification3.0
Jaycean commented on pull request #1245: URL: https://github.com/apache/apisix-dashboard/pull/1245#issuecomment-764447946 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [apisix-dashboard] Jaycean commented on pull request #1245: feat(BE): Export route from OpenAPI Specification3.0
Jaycean commented on pull request #1245: URL: https://github.com/apache/apisix-dashboard/pull/1245#issuecomment-765077174 > > > > Hi, @Jaycean. Need unit test. > > > > > > > > > At present, the function interface needs to export the existing route data and ensure the correctness of the exported data, so I think E2E should be written more perfectly. I think if the simple unit test only tests whether the interface can export data, it does not represent whether the function of the interface is correct, and if the relevant route data is not created, the interface will not work normally use. > > > > > > Not agree with you. > > First of all, unit testing is very important. It helps find bugs easily and quickly if someone changes the code.And unit testing can cover more code than e2e test. > > For example, in this case. > > We need to test these situations at least: > > > > 1. route can't found > > 2. Uris contain "*" > > 3. plugins with `request-validation` > > 4. different route method > >... > > > > > and if the relevant route data is not created, the interface will not work normally use. > > > > > > We can mock data in etcd. > > See https://github.com/apache/apisix-dashboard/blob/master/api/internal/handler/server_info/server_info_test.go for more details. > > @starsz Now I have encountered a problem in unit testing, exporting the data that needs to create route, upstream and service. After reading the example you gave me, I still don't know how to write it. Can you give me some advice. Thks 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [apisix-dashboard] Jaycean commented on pull request #1245: feat(BE): Export route from OpenAPI Specification3.0
Jaycean commented on pull request #1245: URL: https://github.com/apache/apisix-dashboard/pull/1245#issuecomment-765076940 > > > Hi, @Jaycean. Need unit test. > > > > > > At present, the function interface needs to export the existing route data and ensure the correctness of the exported data, so I think E2E should be written more perfectly. I think if the simple unit test only tests whether the interface can export data, it does not represent whether the function of the interface is correct, and if the relevant route data is not created, the interface will not work normally use. > > Not agree with you. > First of all, unit testing is very important. It helps find bugs easily and quickly if someone changes the code.And unit testing can cover more code than e2e test. > For example, in this case. > We need to test these situations at least: > > 1. route can't found > 2. Uris contain "*" > 3. plugins with `request-validation` > 4. different route method >... > > > and if the relevant route data is not created, the interface will not work normally use. > > We can mock data in etcd. > See https://github.com/apache/apisix-dashboard/blob/master/api/internal/handler/server_info/server_info_test.go for more details. Now I have encountered a problem in unit testing, exporting the data that needs to create route, upstream and service. After reading the example you gave me, I still don't know how to write it. Can you give me some advice. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [apisix-dashboard] Jaycean commented on pull request #1245: feat(BE): Export route from OpenAPI Specification3.0
Jaycean commented on pull request #1245: URL: https://github.com/apache/apisix-dashboard/pull/1245#issuecomment-764447946 > it would be better to add three more test cases: > > * export a route created with upstream id > * export a route created with service which is created with upstream id > * export a route created with `basic-auth` plugin enabled > it would be better to add three more test cases: > > * export a route created with upstream id > * export a route created with service which is created with upstream id > * export a route created with `basic-auth` plugin enabled I have added test cases for these three kinds of exports. Auth authentication has added the route export test cases opened by JWT plugin, and the following basic auth and API key will also be added. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [apisix-dashboard] Jaycean commented on pull request #1245: feat(BE): Export route from OpenAPI Specification3.0
Jaycean commented on pull request #1245: URL: https://github.com/apache/apisix-dashboard/pull/1245#issuecomment-757631777 > > > Hi, @Jaycean. Need unit test. > > > > > > At present, the function interface needs to export the existing route data and ensure the correctness of the exported data, so I think E2E should be written more perfectly. I think if the simple unit test only tests whether the interface can export data, it does not represent whether the function of the interface is correct, and if the relevant route data is not created, the interface will not work normally use. > > Not agree with you. > First of all, unit testing is very important. It helps find bugs easily and quickly if someone changes the code.And unit testing can cover more code than e2e test. > For example, in this case. > We need to test these situations at least: > > 1. route can't found > 2. Uris contain "*" > 3. plugins with `request-validation` > 4. different route method >... > > > and if the relevant route data is not created, the interface will not work normally use. > > We can mock data in etcd. > See https://github.com/apache/apisix-dashboard/blob/master/api/internal/handler/server_info/server_info_test.go for more details. Thank you for your advice. I also think unit testing is really necessary. I just thought that this interface test is almost the same as E2E, but unit testing should cover all situations as much as possible 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [apisix-dashboard] Jaycean commented on pull request #1245: feat(BE): Export route from OpenAPI Specification3.0
Jaycean commented on pull request #1245: URL: https://github.com/apache/apisix-dashboard/pull/1245#issuecomment-757598022 > Hi, @Jaycean. Need unit test. At present, the function interface needs to export the existing route data and ensure the correctness of the exported data, so I think E2E should be written more perfectly. I think if the simple unit test only tests whether the interface can export data, it does not represent whether the function of the interface is correct, and if the relevant route data is not created, the interface will not work normally use. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
