Skip to content

pkg: authorization: cleanup tests#22588

Merged
vdemeester merged 1 commit intomoby:masterfrom
runcom:fix-authz-tests
May 13, 2016
Merged

pkg: authorization: cleanup tests#22588
vdemeester merged 1 commit intomoby:masterfrom
runcom:fix-authz-tests

Conversation

@runcom
Copy link
Copy Markdown
Member

@runcom runcom commented May 8, 2016

  • do use use log pkg
  • do not t.Fatal in goroutine
  • cleanups

Signed-off-by: Antonio Murdaca runcom@redhat.com

- do use use log pkg
- do not t.Fatal in goroutine
- cleanups

Signed-off-by: Antonio Murdaca <runcom@redhat.com>
@runcom runcom force-pushed the fix-authz-tests branch from 315abf3 to 6a96684 Compare May 8, 2016 12:18
t.recordedRequest = Request{}
defer r.Body.Close()
body, err := ioutil.ReadAll(r.Body)
body, _ := ioutil.ReadAll(r.Body)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why ignoring errors ? (here and below ?)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

As it was done somewhere else in this test. These specific removals were because you can't log or Fatal here in a goroutine

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

True but you could return a error (in the response) and thus make the test fail if an error occured (and was not wanted) 👼

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

the body is checked in the tests above using reflect.DeepEqual so I think it's safe to ignore the error but just print the body to the response (it will fail the reflect check eventually)

r.HandleFunc("/Plugin.Activate", t.activate)
r.HandleFunc("/"+AuthZApiRequest, t.auth)
r.HandleFunc("/"+AuthZApiResponse, t.auth)
t.listener, _ = net.Listen("tcp", pluginAddress)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I don't have any idea why this line was here though.....

@LK4D4
Copy link
Copy Markdown
Contributor

LK4D4 commented May 12, 2016

@vdemeester is it ok for you?

@vdemeester
Copy link
Copy Markdown
Member

yep 😉

@LK4D4
Copy link
Copy Markdown
Contributor

LK4D4 commented May 12, 2016

LGTM
@vdemeester merge it or move to 4-merge :)

@vdemeester
Copy link
Copy Markdown
Member

LGTM 🐯

@vdemeester vdemeester merged commit 29fe2f3 into moby:master May 13, 2016
@runcom runcom deleted the fix-authz-tests branch May 13, 2016 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants