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

Fix CI slowness and correct execution tests #686

Merged
merged 8 commits into from
Sep 2, 2024
Merged

Conversation

olethanh
Copy link
Collaborator

@olethanh olethanh commented Aug 28, 2024

This PR solve a few issue with the tests:

  • test_create_execution Was VERY VERY slow, more than 10 minutes to run. -> Now it run in less than 1 minutes
  • test_create_execution_online and test_create_execution_legacy were actually running the same test as test_create_execution. (because of a settings contamination) Which compelled the problem and made the whole test suite take 3/4h to run
  • Additionally the code path of running a real program wasn't tested
  • The Workflow name was not correct
  • Execution test of a program were failing on python 3.12

Self proofreading checklist

  • The new code clear, easy to read and well commented.
  • New code does not duplicate the functions of builtin or popular libraries.
  • New classes and functions contain docstrings explaining what they provide.
  • All new code is covered by relevant tests.
  • Documentation has been updated regarding these changes.

Changes

see above

How to test

Everything is in CI

@olethanh olethanh changed the title Disable slow import Investigate why the CI is slow Aug 28, 2024
Copy link

codecov bot commented Aug 28, 2024

Codecov Report

Attention: Patch coverage is 78.57143% with 6 lines in your changes missing coverage. Please review.

Project coverage is 62.66%. Comparing base (cd6463c) to head (15214f4).
Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
src/aleph/vm/hypervisors/firecracker/microvm.py 33.33% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #686      +/-   ##
==========================================
+ Coverage   61.36%   62.66%   +1.30%     
==========================================
  Files          68       68              
  Lines        6012     6029      +17     
  Branches      636      638       +2     
==========================================
+ Hits         3689     3778      +89     
+ Misses       2169     2101      -68     
+ Partials      154      150       -4     

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

Solution: This was due to an import in the test app that is somehow
very slow but only during testing.

Haven't figured out why it is slow, but have implemented a workaround
that delay the import so it's not hit during the tests
@olethanh olethanh changed the title Investigate why the CI is slow Fix CI slowness and correct execution tests Aug 29, 2024
This was due to as settings contamination which made it runn the FAKE_DATA_PROGRAM instead of the real one

Also correct some things that made the test not run (load_update_mesage
instead of get_message)
It was the same name as an other workflow which caused issue in github
@olethanh olethanh marked this pull request as ready for review August 29, 2024 19:54
@olethanh olethanh requested a review from hoh August 29, 2024 20:18
.github/workflows/test-using-pytest.yml Outdated Show resolved Hide resolved
runtimes/aleph-debian-11-python/init1.py Show resolved Hide resolved
src/aleph/vm/hypervisors/firecracker/microvm.py Outdated Show resolved Hide resolved
tests/supervisor/test_execution.py Show resolved Hide resolved
tests/supervisor/test_execution.py Show resolved Hide resolved
tests/supervisor/test_execution.py Outdated Show resolved Hide resolved
src/aleph/vm/hypervisors/firecracker/microvm.py Outdated Show resolved Hide resolved
@olethanh olethanh merged commit 46063fb into main Sep 2, 2024
27 checks passed
@Psycojoker Psycojoker deleted the ol-debug-slow-test branch September 3, 2024 16:16
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.

3 participants