Skip to content
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

test: add consumer e2e test for manager-api #830

Closed
wants to merge 13 commits into from

Conversation

idbeta
Copy link
Contributor

@idbeta idbeta commented Nov 19, 2020

Please answer these questions before submitting a pull request

@codecov-io
Copy link

codecov-io commented Nov 19, 2020

Codecov Report

Merging #830 (34c5b82) into v2.0 (f2ff23a) will decrease coverage by 0.07%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             v2.0     #830      +/-   ##
==========================================
- Coverage   42.64%   42.56%   -0.08%     
==========================================
  Files          18       18              
  Lines        1257     1257              
==========================================
- Hits          536      535       -1     
- Misses        630      631       +1     
  Partials       91       91              
Impacted Files Coverage Δ
api/internal/core/store/store.go 78.28% <0.00%> (-0.66%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f2ff23a...34c5b82. Read the comment docs.

Path: "/hello",
Headers: map[string]string{"apikey": "auth-one"},
ExpectStatus: http.StatusOK,
Sleep: sleepTime, //sleep x millisecond before verify route
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to check body.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to check before consumer created.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, thanks for the suggestion, I already add it now.

api/test/e2e/consumer_test.go Outdated Show resolved Hide resolved
api/test/e2e/consumer_test.go Show resolved Hide resolved
@idbeta idbeta requested a review from nic-chen November 19, 2020 10:01
@idbeta idbeta changed the title test: Add consumer e2e test for manager-api test: add consumer e2e test for manager-api Nov 23, 2020
@moonming moonming added this to the 2.1.1 milestone Nov 24, 2020
@juzhiyuan juzhiyuan added the wait for update 1. Solution 2. Due date 3. Assignees if needed label Nov 25, 2020
"username":"case_8",
"desc": "new consumer"
}`
request, _ := http.NewRequest("PUT", "http://127.0.0.1:8080/apisix/admin/consumers", strings.NewReader(data))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it better to extract public variables? such as BASE_URL = http://127.0.0.1:8080/apisix/admin/consumers

Copy link
Member

@membphis membphis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can close this PR, split this big PR to smaller ones

"github.com/tidwall/gjson"
)

//case 1: add consumer with username
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove useless code

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and pls remove other similar points

ExpectStatus: http.StatusOK,
},
{
caseDesc: "verify route",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

desc: hit route with correct key

Sleep: sleepTime,
},
{
caseDesc: "verify route",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

desc: hit route with incorrect key

ExpectStatus: http.StatusUnauthorized,
ExpectBody: "Invalid API key in request",
Sleep: sleepTime,
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to delete the consumer and check it in dataplane

func TestConsumer_add_consumer_without_username(t *testing.T) {
tests := []HttpTestCase{
{
caseDesc: "create consumer",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bad title.

create consumer without user name

ExpectStatus: http.StatusBadRequest,
},
{
caseDesc: "verify route",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to create the route first

@membphis membphis closed this Nov 26, 2020
@idbeta idbeta deleted the add-e2e-test-for-consumer branch December 9, 2020 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wait for update 1. Solution 2. Due date 3. Assignees if needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants