Skip to content

Add test coverage for middleware#144

Merged
zhouzhuojie merged 1 commit into
masterfrom
zz/add-test-coverage-for-middleware
Jul 10, 2018
Merged

Add test coverage for middleware#144
zhouzhuojie merged 1 commit into
masterfrom
zz/add-test-coverage-for-middleware

Conversation

@zhouzhuojie
Copy link
Copy Markdown
Collaborator

No description provided.

Comment thread pkg/config/middleware_test.go Outdated
Value: "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJmbGFncl91c2VyIjoiMTIzNDU2Nzg5MCJ9.CLXgNEtwPCqCOtUU-KmqDyO8S2wC_G6PZ0tml8DCuNw", // {"flagr_user": "1234567890"}
})
hh.ServeHTTP(res, req)
assert.NotEqual(t, 307, res.Code)
Copy link
Copy Markdown

@rstratton rstratton Jul 10, 2018

Choose a reason for hiding this comment

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

Nit, might be better to assert.Equal the actual value we expect, so that tests document behavior more directly.

Comment thread pkg/config/middleware_test.go Outdated
req, _ := http.NewRequest("GET", "http://localhost:18000/api/v1/flags", nil)

incrCalled := false
monkey.PatchInstanceMethod(
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🐵

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yup, I was mindblown by this testing package monkey, it's so powerful that I don't even need to mock the functions. It replaces a function at runtime.

https://bou.ke/blog/monkey-patching-in-go/

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

That's crazy, so cool!

@zhouzhuojie zhouzhuojie force-pushed the zz/add-test-coverage-for-middleware branch 2 times, most recently from b206ccd to 4068278 Compare July 10, 2018 20:24
@zhouzhuojie zhouzhuojie force-pushed the zz/add-test-coverage-for-middleware branch from 4068278 to 9ff62e0 Compare July 10, 2018 20:31
@zhouzhuojie zhouzhuojie merged commit b46367a into master Jul 10, 2018
@zhouzhuojie zhouzhuojie deleted the zz/add-test-coverage-for-middleware branch July 10, 2018 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants