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

Use new docker image for t8code tests #553

Merged
merged 34 commits into from
Jul 12, 2023
Merged

Use new docker image for t8code tests #553

merged 34 commits into from
Jul 12, 2023

Conversation

sandro-elsweijer
Copy link
Collaborator

@sandro-elsweijer sandro-elsweijer commented May 12, 2023

Describe your changes here:

  1. Uses a docker container to run the tests. This was, we can install all dependencies beforehand in the docker image (e.g. the VTK installation, which needs up to an hour) (Closes Improve the CI by using a precompiled container. #24)
  2. Adds a VTK testing configuration (Closes Add vtk linkage to CI #140)
  3. Changes the OCC and NetCDF tests to the debug configuration, to also catch assertions in the code
  4. Adds checks for the make install -j command (Closes CI does not check make install for t8code #276)
  5. Split up test files for parallel testing. Reduces CI time from 30 min to 22 min
  6. Catches errors which werent catched before (Closes Serial CI job doesn't complain about MPI macros #611 )

#4inarow

All these boxes must be checked by the reviewers before merging the pull request:

As a reviewer please read through all the code lines and make sure that the code is fully understood, bug free, well-documented and well-structured.

General

  • The reviewer executed the new code features at least once and checked the results manually

  • The code follows the t8code coding guidelines

  • New source/header files are properly added to the Makefiles

  • The code is well documented

  • All function declarations, structs/classes and their members have a proper doxygen documentation

  • All new algorithms and data structures are sufficiently optimal in terms of memory and runtime (If this should be merged, but there is still potential for optimization, create a new issue)

Tests

  • The code is covered in an existing or new test case using Google Test

Github action

  • The code compiles without warning in debugging and release mode, with and without MPI (this should be executed automatically in a github action)

  • All tests pass (in various configurations, this should be executed automatically in a github action)

    If the Pull request introduces code that is not covered by the github action (for example coupling with a new library):

    • Should this use case be added to the github action?
    • If not, does the specific use case compile and all tests pass (check manually)

Scripts and Wiki

  • If a new directory with source-files is added, it must be covered by the script/find_all_source_files.scp to check the indentation of these files.
  • New Datatypes are added to t8indent_custom_datatypes.txt
  • If this PR introduces a new feature, it must be covered in an example/tutorial and a Wiki article.

Licence

  • The author added a BSD statement to doc/ (or already has one)

@sandro-elsweijer sandro-elsweijer added the draft Enhance the visibility that this is a draft. label May 12, 2023
@sandro-elsweijer sandro-elsweijer self-assigned this May 12, 2023
@sandro-elsweijer sandro-elsweijer force-pushed the feature-use_docker_in_CI branch from 19099c0 to 36017a8 Compare May 12, 2023 08:38
@sandro-elsweijer sandro-elsweijer force-pushed the feature-use_docker_in_CI branch from 1ad8d0b to b47bf69 Compare May 14, 2023 15:37
@sandro-elsweijer
Copy link
Collaborator Author

The upload of logs was tested here: https://github.com/DLR-AMR/t8code/actions/runs/4973151350

@sandro-elsweijer sandro-elsweijer removed their assignment May 14, 2023
@sandro-elsweijer sandro-elsweijer added CI Continuous Integration and removed draft Enhance the visibility that this is a draft. labels May 14, 2023
@sandro-elsweijer
Copy link
Collaborator Author

The new tests are quite slow because of the docker environment. Maybe somebody has some tips about speeding this up.

@sandro-elsweijer sandro-elsweijer marked this pull request as draft June 12, 2023 14:01
auto-merge was automatically disabled June 12, 2023 14:01

Pull request was converted to draft

@ililikakis ililikakis requested review from Davknapp and removed request for ililikakis June 30, 2023 13:36
@Davknapp
Copy link
Collaborator

Davknapp commented Jul 3, 2023

All runs use "-O3", there should be at least one run with "-O0" in my opinion.

Copy link
Collaborator

@Davknapp Davknapp left a comment

Choose a reason for hiding this comment

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

Looks good in general, please address my comment and have a look at my adaptations.

@Davknapp Davknapp assigned sandro-elsweijer and unassigned Davknapp Jul 3, 2023
@sandro-elsweijer sandro-elsweijer removed their assignment Jul 5, 2023
@sandro-elsweijer sandro-elsweijer requested a review from Davknapp July 5, 2023 10:51
@jmark jmark self-assigned this Jul 10, 2023
@jmark jmark enabled auto-merge July 12, 2023 13:58
@jmark jmark self-requested a review July 12, 2023 13:59
@jmark jmark disabled auto-merge July 12, 2023 14:00
@jmark jmark enabled auto-merge July 12, 2023 14:10
@jmark jmark merged commit d3a35b5 into main Jul 12, 2023
@jmark jmark deleted the feature-use_docker_in_CI branch July 12, 2023 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration
Projects
None yet
4 participants