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

✨ Is3418/validation with ooil test my/osparc/service #3479

Merged
merged 20 commits into from
Oct 31, 2022

Conversation

pcrespov
Copy link
Member

@pcrespov pcrespov commented Oct 27, 2022

What do these changes do?

An important drawback of the https://github.com/ITISFoundation/cookiecutter-osparc-service is the extensive python boilerplate used to validate (i.e. test) the user's service. This PR allows that validation to be run directly from ooil version 1.0.3 executable. Since the latter is in a container, this new command suppresses the need of any boilerplate or python installation on the user's repository.

  • ✨ New command to validate services: ooil test my/osparc/service
    • separate test process: spaws a pytest to run a pre-defined testsuite in service_integration/services/tests
    • isolated: the path to the target service is passed in the command line and transformed into fixtures (see service_integration/services/tests/conftest.py)
    • maintainable: Validations can be extended as the features or requirements on integration evolves by just adding more tests in service_integration/services/tests
  • 🐛 Fixes on ooil Dockerfile:
    • installs git
    • adds non-root user for virtual-environment
  • 🔨 Makefile recipe and scripts/ooil.bash to run executable in a container
  • ⬆️ upgrades pytest to overcome vulnerability GHSA-w596-4wvx-j9j6

Related issue/s

How to test

$ cd packages/service-integration
$ make build
$ make inspect

# or
$ (DOCKER_REGISTRY=local; DOCKER_IMAGE_TAG=production; ./scripts/ooil.bash test --help)
$ (DOCKER_REGISTRY=local; DOCKER_IMAGE_TAG=production; ./scripts/ooil.bash test folder/with/.osparc)

Checklist

@pcrespov pcrespov self-assigned this Oct 27, 2022
@codecov
Copy link

codecov bot commented Oct 27, 2022

Codecov Report

Merging #3479 (23d5254) into master (402a35a) will increase coverage by 0.7%.
The diff coverage is n/a.

❗ Current head 23d5254 differs from pull request most recent head d53cee2. Consider uploading reports for the commit d53cee2 to get more accurate results

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #3479     +/-   ##
========================================
+ Coverage    80.4%   81.2%   +0.7%     
========================================
  Files         688     792    +104     
  Lines       28765   33484   +4719     
  Branches      420     744    +324     
========================================
+ Hits        23150   27206   +4056     
- Misses       5514    6100    +586     
- Partials      101     178     +77     
Flag Coverage Δ
integrationtests 49.9% <0.0%> (?)
unittests 79.9% <0.0%> (-0.6%) ⬇️

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

Impacted Files Coverage Δ
...log/src/simcore_service_catalog/api/routes/dags.py 62.7% <0.0%> (-11.7%) ⬇️
...src/simcore_service_catalog/api/routes/services.py 56.5% <0.0%> (-4.4%) ⬇️
...simcore_service_director_v2/modules/node_rights.py 98.1% <0.0%> (-1.0%) ⬇️
...es/catalog/src/simcore_service_catalog/api/root.py 100.0% <0.0%> (ø)
...ice_director_v2/models/domains/dynamic_services.py 100.0% <0.0%> (ø)
...ector_v2/modules/dynamic_sidecar/scheduler/task.py 84.1% <0.0%> (ø)
...es/storage/src/simcore_service_storage/s3_utils.py
...storage/src/simcore_service_storage/datcore_dsm.py
...s/storage/src/simcore_service_storage/constants.py
...es/storage/src/simcore_service_storage/settings.py
... and 216 more

@pcrespov pcrespov force-pushed the is3418/ooil-updates branch from e28123e to a824c0a Compare October 28, 2022 10:54
@pcrespov pcrespov changed the title WIP: Is3418/ooil updates Is3418/ooil updates Oct 30, 2022
@pcrespov pcrespov changed the title Is3418/ooil updates Is3418/validation with ooil test my/osparc/service Oct 30, 2022
@pcrespov pcrespov changed the title Is3418/validation with ooil test my/osparc/service ✨ Is3418/validation with ooil test my/osparc/service Oct 30, 2022
@pcrespov pcrespov marked this pull request as ready for review October 30, 2022 21:57
@pcrespov pcrespov added the a:ooil integration-library or ooil label Oct 30, 2022
@pcrespov pcrespov added this to the Katherine Switzer milestone Oct 30, 2022
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.

So if I get that correctly, this is one more step to having an almost empty cookie cutter? pretty nice work!

@pcrespov
Copy link
Member Author

So if I get that correctly, this is one more step to having an almost empty cookie cutter? pretty nice work!

not literally empty but yes, no more boilerplate around the actual service code.

@pcrespov pcrespov force-pushed the is3418/ooil-updates branch 2 times, most recently from c5af2de to 8d68f0f Compare October 31, 2022 12:49
@pcrespov pcrespov force-pushed the is3418/ooil-updates branch from 8d68f0f to d53cee2 Compare October 31, 2022 13:53
@sonarcloud
Copy link

sonarcloud bot commented Oct 31, 2022

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

Copy link
Collaborator

@elisabettai elisabettai left a comment

Choose a reason for hiding this comment

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

Haven't check the details yet, but this looks great! Will need a tutorial/demo when it's finished. ☺️

@pcrespov pcrespov merged commit 17d49db into ITISFoundation:master Oct 31, 2022
@pcrespov pcrespov deleted the is3418/ooil-updates branch October 31, 2022 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:ooil integration-library or ooil
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants