xmidt-org / wrp-go

go implementation of the Web Routing Protocol
Apache License 2.0
4 stars 7 forks source link

bug: Patch outdated error assert for `testWRPHandlerDecodeError` #73

Closed denopink closed 2 years ago

denopink commented 2 years ago

Overview

tl;rd

Update testWRPHandlerDecodeError to handle its expected error as a httpError.

Explanation `wrpHandler.ServeHTTP` now returns a 400 httpError when it's decoder returns an error: https://github.com/xmidt-org/wrp-go/blob/4b275411c7aa52e1a537cc130f3b0f37e526c978/wrphttp/handler.go#L114-L122 But `testWRPHandlerDecodeError` expects the non-httpError version of `actualErr`: https://github.com/xmidt-org/wrp-go/blob/4b275411c7aa52e1a537cc130f3b0f37e526c978/wrphttp/handler_test.go#L205-L217
Type of Change(s) - Bug fix (non-breaking change which fixes an issue) - All new and existing tests passed.
Module Unit Testing: [100% PASSING] Passed all package unit tests.
PR Affecting Unit Testing: handler_test.go [100% PASSING] ```console Running tool: /usr/local/bin/go test -timeout 30s -run ^(TestHandlerFunc|TestWithErrorEncoder|TestWithNewResponseWriter|TestWithDecoder|TestWithBefore|TestNewHTTPHandler|TestWRPHandler)$ github.com/xmidt-org/wrp-go/v3/wrphttp === RUN TestHandlerFunc --- PASS: TestHandlerFunc (0.00s) === RUN TestWithErrorEncoder === RUN TestWithErrorEncoder/Default === RUN TestWithErrorEncoder/Custom --- PASS: TestWithErrorEncoder (0.00s) --- PASS: TestWithErrorEncoder/Default (0.00s) --- PASS: TestWithErrorEncoder/Custom (0.00s) === RUN TestWithNewResponseWriter === RUN TestWithNewResponseWriter/Default === RUN TestWithNewResponseWriter/Custom --- PASS: TestWithNewResponseWriter (0.00s) --- PASS: TestWithNewResponseWriter/Default (0.00s) --- PASS: TestWithNewResponseWriter/Custom (0.00s) === RUN TestWithDecoder === RUN TestWithDecoder/Default === RUN TestWithDecoder/Custom --- PASS: TestWithDecoder (0.00s) --- PASS: TestWithDecoder/Default (0.00s) --- PASS: TestWithDecoder/Custom (0.00s) === RUN TestWithBefore === RUN TestWithBefore/0 === RUN TestWithBefore/1 === RUN TestWithBefore/2 === RUN TestWithBefore/3 --- PASS: TestWithBefore (0.00s) --- PASS: TestWithBefore/0 (0.00s) --- PASS: TestWithBefore/1 (0.00s) --- PASS: TestWithBefore/2 (0.00s) --- PASS: TestWithBefore/3 (0.00s) === RUN TestNewHTTPHandler --- PASS: TestNewHTTPHandler (0.00s) === RUN TestWRPHandler === RUN TestWRPHandler/ServeHTTP === RUN TestWRPHandler/ServeHTTP/DecodeError === RUN TestWRPHandler/ServeHTTP/ResponseWriterError === RUN TestWRPHandler/ServeHTTP/Success /Users/odc/Documents/GitHub/xmidt-org/wrp-go/wrphttp/handler_test.go:345: PASS: ServeWRP(mock.argumentMatcher,mock.argumentMatcher) --- PASS: TestWRPHandler (0.00s) --- PASS: TestWRPHandler/ServeHTTP (0.00s) --- PASS: TestWRPHandler/ServeHTTP/DecodeError (0.00s) --- PASS: TestWRPHandler/ServeHTTP/ResponseWriterError (0.00s) --- PASS: TestWRPHandler/ServeHTTP/Success (0.00s) PASS ok github.com/xmidt-org/wrp-go/v3/wrphttp 0.229s > Test run finished at 4/26/2022, 2:19:47 PM < ```
codecov[bot] commented 2 years ago

Codecov Report

Merging #73 (3cb6fbf) into main (0a1cb0b) will increase coverage by 0.17%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #73      +/-   ##
==========================================
+ Coverage   48.45%   48.63%   +0.17%     
==========================================
  Files          16       17       +1     
  Lines        3178     3189      +11     
==========================================
+ Hits         1540     1551      +11     
  Misses       1468     1468              
  Partials      170      170              
Impacted Files Coverage Δ
wrphttp/decoders.go 72.72% <100.00%> (+1.75%) :arrow_up:
wrphttp/errors.go 100.00% <100.00%> (ø)
wrphttp/handler.go 100.00% <100.00%> (ø)

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 4b27541...3cb6fbf. Read the comment docs.

denopink commented 2 years ago

@kristinapathak I think we're ready for this one. Once we merge this pr, I'll rebase #74 with this pr's test fixes