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

Improving CICD tests stability #1481

Merged
merged 7 commits into from
Sep 15, 2022
Merged

Conversation

germa89
Copy link
Collaborator

@germa89 germa89 commented Sep 9, 2022

Following duck typing....

@germa89 germa89 added Enhancement Improve any current implemented feature CI/CD Related with CICD, Github Actions, etc labels Sep 9, 2022
@germa89 germa89 added this to the v0.63.3 milestone Sep 9, 2022
@germa89 germa89 self-assigned this Sep 9, 2022
@github-actions github-actions bot added Maintenance General maintenance of the repo (libraries, cicd, etc) New Feature Request or proposal for a new feature labels Sep 9, 2022
@germa89
Copy link
Collaborator Author

germa89 commented Sep 9, 2022

I'm going to assume that --restart docker policy works fine, hence it just need a bit of time for the image to get back.

I mean, playing locally, I can do the following:

  • Start a docker MAPDL container
  • Connect to it.
  • Run mapdl.prep7()
  • Stop the container
  • Run mapdl.prep7() and have the MapdlExitedError.
  • Remove the container completely
  • Start another container in the same port.
  • Run mapdl.prep7() successfully (not issuing any other command).

Hence, if I retry the failing tests, giving some extra time for the container to run, the most of the MAPDL stabilities should be fixed.

Well... not fixed, rather mitigated.

@codecov
Copy link

codecov bot commented Sep 9, 2022

Codecov Report

Merging #1481 (6f413ff) into main (3d21ac6) will increase coverage by 0.26%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1481      +/-   ##
==========================================
+ Coverage   80.32%   80.58%   +0.26%     
==========================================
  Files          43       43              
  Lines        6779     6789      +10     
==========================================
+ Hits         5445     5471      +26     
+ Misses       1334     1318      -16     

@germa89
Copy link
Collaborator Author

germa89 commented Sep 9, 2022

It works... kind of...

============================= slowest 10 durations =============================
21.92s call     tests/test_grpc.py::test_read_input_file[False-full26.dat]
15.16s call     tests/test_grpc.py::test_read_input_file[False-static.dat]
6.34s setup    tests/test_mapdl.py::test_get_variable_nsol_esol_wrappers[False]
3.09s setup    tests/test_commands.py::test_output_listing[False-prnsol-args0]
2.18s call     tests/test_grpc.py::test_download_project[False]
2.11s call     tests/test_mapdl.py::test_cyclic_solve[False]
2.09s setup    tests/test_post.py::test_plot_nodal_contact_friction_stress[False]
2.08s setup    tests/test_post.py::test_time_values[False]
2.08s setup    tests/test_mesh_grpc.py::test_tshape_key[False]
2.03s setup    tests/test_post.py::test_nodal_contact_friction_stress[False]
========================= PyMAPDL Pytest short summary =========================
Error:  test_close[False] - E           ansys.mapdl.core.errors.MapdlExitedError: MAPDL server connection terminated
= 897 passed, 220 skipped, 108 xfailed, 2 xpassed, 1 error, 6 rerun in 191.56s (0:03:11) =
Error: Process completed with exit code 1.

I think the stability has been improved overall.

I can still do more, like checking the health of the docker image before running a test (that's going to be fun)

@akaszynski
Copy link
Collaborator

I can still do more, like checking the health of the docker image before running a test (that's going to be fun)

Good idea. You can do that with a fixture that uses autorun.

tests/test_grpc.py Outdated Show resolved Hide resolved
@germa89
Copy link
Collaborator Author

germa89 commented Sep 12, 2022

I think this is kind of working:

https://github.com/pyansys/pymapdl/runs/8308521660?check_suite_focus=true#step:10:902

Mon, 12 Sep 2022 14:54:32 GMT tests/test_post.py::test_time_frequency_values[False] RERUN              [ 62%]
Mon, 12 Sep 2022 14:54:34 GMT tests/test_post.py::test_time_frequency_values[False] RERUN              [ 62%]
Mon, 12 Sep 2022 14:54:37 GMT tests/test_post.py::test_time_frequency_values[False] RERUN              [ 62%]
Mon, 12 Sep 2022 14:54:39 GMT tests/test_post.py::test_time_frequency_values[False] RERUN              [ 62%]
Mon, 12 Sep 2022 14:54:41 GMT tests/test_post.py::test_time_frequency_values[False] RERUN              [ 62%]
Mon, 12 Sep 2022 14:54:41 GMT tests/test_post.py::test_time_frequency_values[False] ERROR              [ 62%]
Mon, 12 Sep 2022 14:54:43 GMT tests/test_post.py::test_time_values[False] RERUN                        [ 63%]
Mon, 12 Sep 2022 14:54:45 GMT tests/test_post.py::test_time_values[False] RERUN                        [ 63%]
Mon, 12 Sep 2022 14:54:48 GMT tests/test_post.py::test_time_values[False] PASSED                       [ 63%]

Still I got an issue here. I could try to use a higher delay time (3seconds?) and/or check docker health before running the command.

First option is easier.... second is more fun.

@germa89 germa89 marked this pull request as draft September 12, 2022 16:01
@germa89
Copy link
Collaborator Author

germa89 commented Sep 12, 2022

I think I'm going to use a higher delay (3s) and see how it is going.

I will keep the docker health check for another occasion/PR.

@germa89 germa89 marked this pull request as ready for review September 12, 2022 16:19
Copy link
Collaborator Author

@germa89 germa89 left a comment

Choose a reason for hiding this comment

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

Looks AMAZING to me

image

@germa89 germa89 merged commit bc68ddb into main Sep 15, 2022
@germa89 germa89 deleted the feat/improving-CICD-tests-stability branch September 15, 2022 10:52
germa89 added a commit that referenced this pull request Oct 11, 2022
* First approach

* Changing time to 2 seconds

* Upload the requirements

* Preparing flag.

* Update tests/test_grpc.py

* Increasing delays and reruns.
germa89 added a commit that referenced this pull request Oct 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI/CD Related with CICD, Github Actions, etc Dependencies Enhancement Improve any current implemented feature Maintenance General maintenance of the repo (libraries, cicd, etc) New Feature Request or proposal for a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants