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

Support range definition for response codes. #65

Merged
merged 3 commits into from
Apr 21, 2024

Conversation

k1LoW
Copy link
Contributor

@k1LoW k1LoW commented Apr 16, 2024

Hi all.

This fix supports the range definition for response codes.

ref: https://swagger.io/docs/specification/describing-responses/

Copy link

codecov bot commented Apr 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.71%. Comparing base (f866c7b) to head (8733614).
Report is 9 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #65      +/-   ##
==========================================
+ Coverage   98.96%   99.71%   +0.74%     
==========================================
  Files          19       19              
  Lines        1156     1733     +577     
==========================================
+ Hits         1144     1728     +584     
+ Misses         12        5       -7     
Flag Coverage Δ
unittests 99.71% <100.00%> (+0.74%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@k1LoW
Copy link
Contributor Author

k1LoW commented Apr 18, 2024

Hi @daveshanley
codecov/project is FAIL, is there anything I need to do additionally to address this?

@daveshanley
Copy link
Member

Hi @k1LoW yeah, there are some indirect changes that prevent some code from executing

https://app.codecov.io/gh/pb33f/libopenapi-validator/pull/65/blob/responses/validate_response.go

As you can see there are two branches that now no-longer run, would you be able to address this?

Thanks

@k1LoW
Copy link
Contributor Author

k1LoW commented Apr 18, 2024

@daveshanley

Report is 9 commits behind head on main. ( ref: #65 (comment) )

The current main branch is b3f6e33.
Comparing coverage with it, the coverage range seems to be the same.

Are you saying that there is a problem with the commitments already taken in?

@daveshanley
Copy link
Member

From what I understand this is saying (based on my experience with this codecov tool) is that the new code added is indirectly causing existing code to not run (https://app.codecov.io/gh/pb33f/libopenapi-validator/pull/65/blob/responses/validate_response.go) You can see the red lines that are no-longer running as part of the test suite.

This is an indirect change, meaning that the code no-longer runs, based on your changes, even if you did not change tests that call that code.

@daveshanley
Copy link
Member

You can see in this PR for example: pb33f/libopenapi#274 I am battling the code coverage over time after fixing bugs and regressions, sometimes the indirect coverage drop is a head scratcher - but it means that code that was running before, is no-longer covered by a test.

@daveshanley
Copy link
Member

Hi @k1LoW yeah, there are some indirect changes that prevent some code from executing

https://app.codecov.io/gh/pb33f/libopenapi-validator/pull/65/blob/responses/validate_response.go

As you can see there are two branches that now no-longer run, would you be able to address this?

Thanks

What I mean by branches is 'code paths' vs git branches, there are two blocks of code that are marked red, which means they are not covered by any testing anymore.

@k1LoW
Copy link
Contributor Author

k1LoW commented Apr 19, 2024

It is already not covered two branches of validate_response.go in the main.

ref: https://app.codecov.io/gh/pb33f/libopenapi-validator/blob/main/responses%2Fvalidate_response.go

Coverage total and Coverage per files

# /src/github.com/pb33f/libopenapi-validator (main)> go test ./... -coverprofile=coverage.out -covermode=count
        github.com/pb33f/libopenapi-validator/errors            coverage: 0.0% of statements
        github.com/pb33f/libopenapi-validator/helpers           coverage: 0.0% of statements
        github.com/pb33f/libopenapi-validator/schema_validation/openapi_schemas         coverage: 0.0% of statements
ok      github.com/pb33f/libopenapi-validator   0.530s  coverage: 51.9% of statements
ok      github.com/pb33f/libopenapi-validator/parameters        0.605s  coverage: 91.0% of statements
ok      github.com/pb33f/libopenapi-validator/paths     0.824s  coverage: 36.8% of statements
ok      github.com/pb33f/libopenapi-validator/requests  1.453s  coverage: 36.0% of statements
ok      github.com/pb33f/libopenapi-validator/responses 0.971s  coverage: 40.7% of statements
ok      github.com/pb33f/libopenapi-validator/schema_validation 1.241s  coverage: 33.9% of statements
# /src/github.com/pb33f/libopenapi-validator (main)> octocov

            main (b3f6e33)
----------------------------
  Coverage           96.0%

# /src/github.com/pb33f/libopenapi-validator (main)> octocov ls-files
100.0% [    4/4] errors/error_utilities.go
100.0% [  50/50] errors/parameter_errors.go
100.0% [    6/6] errors/request_errors.go
100.0% [  18/18] errors/response_errors.go
 11.1% [    1/9] errors/validation_error.go
 78.3% [  18/23] helpers/operation_utilities.go
 92.4% [134/145] helpers/parameter_utilities.go
100.0% [  54/54] parameters/cookie_parameters.go
100.0% [  63/63] parameters/header_parameters.go
100.0% [    3/3] parameters/parameters.go
 99.3% [139/140] parameters/path_parameters.go
100.0% [  93/93] parameters/query_parameters.go
 98.6% [  69/70] parameters/validate_parameter.go
100.0% [  54/54] parameters/validate_security.go
100.0% [  93/93] parameters/validation_functions.go
 99.3% [143/144] paths/paths.go
100.0% [    3/3] requests/request_body.go
100.0% [  39/39] requests/validate_body.go
100.0% [  61/61] requests/validate_request.go
100.0% [    3/3] responses/response_body.go
100.0% [  52/52] responses/validate_body.go
 90.9% [  60/66] responses/validate_response.go
100.0% [  16/16] schema_validation/locate_schema_property.go
  0.0% [   0/24] schema_validation/openapi_schemas/load_schema.go
100.0% [  32/32] schema_validation/validate_document.go
100.0% [  83/83] schema_validation/validate_schema.go
 98.3% [119/121] validator.go
# /src/github.com/pb33f/libopenapi-validator (main)> git switch range-response-code
Switched to branch 'range-response-code'
# /src/github.com/pb33f/libopenapi-validator (range-response-code)> go test ./... -coverprofile=coverage.out -covermode=count
        github.com/pb33f/libopenapi-validator/helpers           coverage: 0.0% of statements
        github.com/pb33f/libopenapi-validator/errors            coverage: 0.0% of statements
        github.com/pb33f/libopenapi-validator/schema_validation/openapi_schemas         coverage: 0.0% of statements
ok      github.com/pb33f/libopenapi-validator   0.432s  coverage: 51.9% of statements
ok      github.com/pb33f/libopenapi-validator/parameters        0.800s  coverage: 91.0% of statements
ok      github.com/pb33f/libopenapi-validator/paths     0.700s  coverage: 36.8% of statements
ok      github.com/pb33f/libopenapi-validator/requests  0.867s  coverage: 36.0% of statements
ok      github.com/pb33f/libopenapi-validator/responses 0.477s  coverage: 41.0% of statements
ok      github.com/pb33f/libopenapi-validator/schema_validation 0.595s  coverage: 33.9% of statements
# /src/github.com/pb33f/libopenapi-validator (range-response-code)> octocov

            range-response-code (a80bbb4)
-------------------------------------------
  Coverage                          96.0%

# /src/github.com/pb33f/libopenapi-validator (range-response-code)> octocov ls-files
100.0% [    4/4] errors/error_utilities.go
100.0% [  50/50] errors/parameter_errors.go
100.0% [    6/6] errors/request_errors.go
100.0% [  18/18] errors/response_errors.go
 11.1% [    1/9] errors/validation_error.go
 78.3% [  18/23] helpers/operation_utilities.go
 92.4% [134/145] helpers/parameter_utilities.go
100.0% [  54/54] parameters/cookie_parameters.go
100.0% [  63/63] parameters/header_parameters.go
100.0% [    3/3] parameters/parameters.go
 99.3% [139/140] parameters/path_parameters.go
100.0% [  93/93] parameters/query_parameters.go
 98.6% [  69/70] parameters/validate_parameter.go
100.0% [  54/54] parameters/validate_security.go
100.0% [  93/93] parameters/validation_functions.go
 99.3% [143/144] paths/paths.go
100.0% [    3/3] requests/request_body.go
100.0% [  39/39] requests/validate_body.go
100.0% [  61/61] requests/validate_request.go
100.0% [    3/3] responses/response_body.go
100.0% [  54/54] responses/validate_body.go
 90.9% [  60/66] responses/validate_response.go
100.0% [  16/16] schema_validation/locate_schema_property.go
  0.0% [   0/24] schema_validation/openapi_schemas/load_schema.go
100.0% [  32/32] schema_validation/validate_document.go
100.0% [  83/83] schema_validation/validate_schema.go
 98.3% [119/121] validator.go
# /src/github.com/pb33f/libopenapi-validator (range-response-code)>

Also, I didn't add 577 lines of code...

CleanShot 2024-04-19 at 09 21 09@2x

What should I do? 💦

@k1LoW
Copy link
Contributor Author

k1LoW commented Apr 19, 2024

If you will allow me, I can contribute to the coverage by adding tests that are not related to this fix!

@daveshanley
Copy link
Member

If you will allow me, I can contribute to the coverage by adding tests that are not related to this fix!

I would be most grateful! Thank would be very kind of you.

@k1LoW
Copy link
Contributor Author

k1LoW commented Apr 21, 2024

Hi @daveshanley. I added tests for validate_response.go.

@daveshanley
Copy link
Member

@k1LoW You are a superstar! thank you so very much for all your contributions.

Copy link
Member

@daveshanley daveshanley left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

@daveshanley daveshanley merged commit 016b8cd into pb33f:main Apr 21, 2024
3 checks passed
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