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

♻️ Is3626/api-server test tools and coverage #4149

Merged

Conversation

pcrespov
Copy link
Member

@pcrespov pcrespov commented Apr 24, 2023

What do these changes do?

This PR focus on enhancing test coverage of the api-server before adding extra functionality

  • ✅ fixes respx fixtures created (and disabled) in ✨ Is3940/public api can run a job on a given cluster (part 1) #3958
  • ✅ creates a mechanism that captures and dumps http requests from the api-server to any of the backend services.
    • These records can then be used to mock responses to specific workflows. In addition the request captures will be validated against openapi specs to guarantee they are up-to-date.
    • This mechanism is only available for development and controlled via API_SERVER_DEV_HTTP_CALLS_LOGS_PATH
    • For details see utils/http_calls_capture.py and HttpxClientWithCaptures in utils/client_base.py
  • ♻️ refactors settings, _meta, plugins
  • ✅ increase coverage of some modules (e.g. CLI, settings, etc)

⚠️ devops

  • New API_SERVER_DEV_HTTP_CALLS_LOGS_PATH env variable is only for local development and should never be set elsewhere (a validation check is in the code as precaution )

Related issue/s

How to test

cd api-server
make install-dev
make test-dev

@pcrespov pcrespov changed the title Is3626/apiserver increase coverage WIP: Is3626/apiserver increase coverage Apr 24, 2023
@codecov
Copy link

codecov bot commented Apr 24, 2023

Codecov Report

Merging #4149 (7aa068e) into master (f1fefe2) will decrease coverage by 7.5%.
The diff coverage is 70.3%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #4149      +/-   ##
=========================================
- Coverage    85.4%   78.0%    -7.5%     
=========================================
  Files         955     700     -255     
  Lines       41398   31476    -9922     
  Branches      952     219     -733     
=========================================
- Hits        35385   24568   -10817     
- Misses       5797    6849    +1052     
+ Partials      216      59     -157     
Flag Coverage Δ
integrationtests 67.2% <ø> (-0.1%) ⬇️
unittests 74.7% <70.3%> (-7.7%) ⬇️

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

Impacted Files Coverage Δ
...simcore_service_api_server/plugins/remote_debug.py 28.5% <ø> (ø)
.../src/simcore_service_api_server/plugins/storage.py 53.1% <7.6%> (ø)
...rc/simcore_service_api_server/utils/client_base.py 63.9% <41.1%> (-26.4%) ⬇️
...rc/simcore_service_api_server/plugins/webserver.py 77.2% <44.4%> (ø)
...core_service_api_server/api/routes/solvers_jobs.py 58.7% <58.3%> (+11.4%) ⬆️
.../simcore_service_api_server/plugins/director_v2.py 70.0% <60.0%> (ø)
.../src/simcore_service_api_server/plugins/catalog.py 63.6% <63.6%> (ø)
...c/simcore_service_api_server/api/routes/solvers.py 50.7% <66.6%> (ø)
...ore_service_api_server/utils/http_calls_capture.py 84.6% <84.6%> (ø)
...e_service_api_server/api/dependencies/webserver.py 93.5% <90.0%> (-0.3%) ⬇️
... and 11 more

... and 406 files with indirect coverage changes

@pcrespov pcrespov force-pushed the is3626/apiserver-increase-coverage branch from cabd9b9 to 783159b Compare April 24, 2023 15:56
@pcrespov pcrespov self-assigned this Apr 25, 2023
@pcrespov pcrespov added the a:apiserver api-server service label Apr 25, 2023
@pcrespov pcrespov added this to the Jelly Beans milestone Apr 25, 2023
@pcrespov pcrespov force-pushed the is3626/apiserver-increase-coverage branch 3 times, most recently from 855aa56 to 3538da5 Compare April 27, 2023 08:09
@pcrespov pcrespov marked this pull request as ready for review April 27, 2023 08:21
@pcrespov pcrespov changed the title WIP: Is3626/apiserver increase coverage Is3626/api-server test tools and coverage Apr 27, 2023
@pcrespov pcrespov changed the title Is3626/api-server test tools and coverage ♻️ Is3626/api-server test tools and coverage Apr 27, 2023
@pcrespov pcrespov marked this pull request as draft April 27, 2023 08:25
@pcrespov pcrespov marked this pull request as ready for review April 27, 2023 09:00
@pcrespov pcrespov force-pushed the is3626/apiserver-increase-coverage branch from e0ea34c to c879e8e Compare April 27, 2023 11:26
Copy link
Member

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

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

looking promising. I will need you to show how that works.

Copy link
Contributor

@GitHK GitHK left a comment

Choose a reason for hiding this comment

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

👍 Nice!

One question: Will any of the responses you capture be added as a test and validated against in the future? I have not seen them being added here.

@pcrespov
Copy link
Member Author

Will any of the responses you capture be added as a test and validated against in the future? I have not seen them being added here.

Yes, that is what I meant in the description by "... In addition the request captures will be validated against openapi specs to guarantee they are up-to-date...
For the moment this PR has only the "tooling" to create those records but they are still not integrated as tests. This will come in subsequent PRs as I increaase the functionality of the API

@codeclimate
Copy link

codeclimate bot commented Apr 27, 2023

Code Climate has analyzed commit 7aa068e and detected 0 issues on this pull request.

View more on Code Climate.

@sonarcloud
Copy link

sonarcloud bot commented Apr 27, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@pcrespov pcrespov requested a review from sanderegg April 27, 2023 12:46
@pcrespov pcrespov added the t:maintenance Some planned maintenance work label Apr 27, 2023
@pcrespov pcrespov merged commit ba2217c into ITISFoundation:master Apr 27, 2023
@pcrespov pcrespov deleted the is3626/apiserver-increase-coverage branch April 27, 2023 13:59
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request May 30, 2023
24 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:apiserver api-server service t:maintenance Some planned maintenance work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants