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

Correct handling of mime type #405

Merged
merged 14 commits into from
Apr 25, 2018
Merged

Correct handling of mime type #405

merged 14 commits into from
Apr 25, 2018

Conversation

RaeesBhatti
Copy link
Contributor

No description provided.

@codecov
Copy link

codecov bot commented Mar 30, 2018

Codecov Report

Merging #405 into master will decrease coverage by 0.34%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #405      +/-   ##
==========================================
- Coverage   64.19%   63.85%   -0.35%     
==========================================
  Files          29       29              
  Lines        1648     1649       +1     
==========================================
- Hits         1058     1053       -5     
- Misses        547      553       +6     
  Partials       43       43
Impacted Files Coverage Δ
router/event.go 80.43% <66.66%> (-4.01%) ⬇️
router/router.go 57.94% <0%> (-1.33%) ⬇️

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 2a71365...5e0fadc. Read the comment docs.

@RaeesBhatti RaeesBhatti changed the title Check for prefix of mime type Correct handling of mime type Mar 30, 2018
@RaeesBhatti RaeesBhatti removed the request for review from mthenw March 30, 2018 13:17
@RaeesBhatti RaeesBhatti requested a review from mthenw April 3, 2018 18:16
.travis.yml Outdated
@@ -12,7 +12,7 @@ install:
script:
- go get -u github.com/alecthomas/gometalinter
- gometalinter --install --force
- gometalinter --vendor --fast --disable=gotype --disable=vetshadow --disable=gas --skip=mock ./...
- gometalinter --vendor --fast --cyclo-over 12 --disable=gotype --disable=vetshadow --disable=gas --skip=mock ./...
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that we don't need this change.

@@ -116,7 +126,14 @@ func TestRouterServeHTTP_Encoding(t *testing.T) {
}
json.NewDecoder(r.Body).Decode(&testevent)

assert.Equal(t, test["expected"], testevent.Data.(map[string]interface{})["body"])
if strings.HasPrefix(test["content-type"], "application/json") {
Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't have to be like that. There are few issues in this test.

Test cases should not be defined as []map[string]string it should be inline struct

[]struct {
		requestBody     string
		contentType     string
		expectedPayload interface{}
	}

thanks to that we can have string or map[string]interface{} as expectedPayload. Then we don't need to change this line at all.

It looks a bit messy that we have those test cases defined inside the test function. Can you move it to the bottom of this file?

@mthenw mthenw merged commit c00a99f into master Apr 25, 2018
@mthenw mthenw deleted the mimePrefix branch April 25, 2018 09:30
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.

None yet

2 participants