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

Add tests of simtools-prod containers #713

Merged
merged 42 commits into from
Dec 15, 2023
Merged

Add tests of simtools-prod containers #713

merged 42 commits into from
Dec 15, 2023

Conversation

GernotMaier
Copy link
Contributor

@GernotMaier GernotMaier commented Dec 6, 2023

Add a test for the simtools-prod container to ensure that everything is correctly installed. For this, run a single application test using the database and sim_telarray (simtools-simulate-prod).

This required to separate the steps of generating the container and pushing.

The successful github action workflow for the container generation do not appear below, it is:

@GernotMaier GernotMaier self-assigned this Dec 6, 2023
@GernotMaier GernotMaier linked an issue Dec 6, 2023 that may be closed by this pull request
Copy link

codecov bot commented Dec 6, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a5bd97a) 82.18% compared to head (88ebde4) 82.22%.
Report is 1 commits behind head on main.

❗ Current head 88ebde4 differs from pull request most recent head c93b4e9. Consider uploading reports for the commit c93b4e9 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #713      +/-   ##
==========================================
+ Coverage   82.18%   82.22%   +0.04%     
==========================================
  Files          42       41       -1     
  Lines        6196     6210      +14     
==========================================
+ Hits         5092     5106      +14     
  Misses       1104     1104              

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

@GernotMaier GernotMaier marked this pull request as ready for review December 14, 2023 14:52
@GernotMaier
Copy link
Contributor Author

And I notice now that the docker-simtools test start when pressing "read for review".

Copy link
Contributor

@orelgueta orelgueta left a comment

Choose a reason for hiding this comment

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

You are using the config file that comes in tests/resources for running, but do we expect that the production container comes with the tests directory? It probably shouldn't, no?

If we decide to remove tests from this container, we can just provide the arguments directly, they are not too many.

I approve anyway cause the solution works for now.

(As a side note, the amount of commits and their messages for such a small change show how much of a pain it is to work on docker stuff.)

@GernotMaier
Copy link
Contributor Author

No, the tests directory is not part of the container. It is available in the workflow, because we need to do a checkout of simtools to get the Dockerfile. Using the config file simplifies this - and if we change any of the command line parameters we do not have to worry about this test (unless we change the --config parameter.

@orelgueta
Copy link
Contributor

Oh yea, I forgot this is part of building the container and therefore we need to checkout simtools anyway. All good then.

@GernotMaier GernotMaier merged commit c667f1e into main Dec 15, 2023
15 checks passed
@GernotMaier GernotMaier deleted the test-docker-prod branch December 15, 2023 08:04
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.

Add tests that build docker images are working well
2 participants