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

refactor(test): speed up e2e tests #4302

Merged
merged 1 commit into from
Jun 25, 2024

Conversation

ndr-brt
Copy link
Member

@ndr-brt ndr-brt commented Jun 25, 2024

What this PR changes/adds

Export classpath loading from EmbeddedRuntime

Why it does that

Classpath loading is a time consuming operation (approx 14 seconds first time it runs, 4-5 seconds other times) as it needs to invoke gradle wrapper and the printClasspath task. By doing it just once for every runtime in the e2e-transfer-test makes tests run faster (approx 1 minute less than before)

Further notes

List other areas of code that have changed but are not necessarily linked to the main feature. This could be method
signature changes, package declarations, bugs that were encountered and were fixed inline, etc.

Linked Issue(s)

Closes # <-- insert Issue number if one exists

Please be sure to take a look at the contributing guidelines and our etiquette for pull requests.

@ndr-brt ndr-brt added the refactoring Cleaning up code and dependencies label Jun 25, 2024
@codecov-commenter
Copy link

codecov-commenter commented Jun 25, 2024

Codecov Report

Attention: Patch coverage is 0% with 33 lines in your changes missing coverage. Please review.

Project coverage is 75.16%. Comparing base (7f20ba5) to head (1b63226).
Report is 325 commits behind head on main.

Files Patch % Lines
.../eclipse/edc/junit/extensions/ClasspathReader.java 0.00% 27 Missing ⚠️
.../eclipse/edc/junit/extensions/EmbeddedRuntime.java 0.00% 6 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4302      +/-   ##
==========================================
+ Coverage   71.74%   75.16%   +3.42%     
==========================================
  Files         919     1052     +133     
  Lines       18457    21094    +2637     
  Branches     1037     1180     +143     
==========================================
+ Hits        13242    15856    +2614     
+ Misses       4756     4724      -32     
- Partials      459      514      +55     

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

@ndr-brt ndr-brt requested a review from wolf4ood June 25, 2024 12:23
@ndr-brt ndr-brt added the build Anything related to the CI/CD Pipeline on Github Actions label Jun 25, 2024
@ndr-brt ndr-brt marked this pull request as ready for review June 25, 2024 12:24
@paullatzelsperger
Copy link
Member

paullatzelsperger commented Jun 25, 2024

i'm wondering: the improvement consists of not doing the CP scanning in the boot() method, but in the constructor, right? so it would only come into play when the runtime is initialized on every method, but not on every class, correct?

@ndr-brt
Copy link
Member Author

ndr-brt commented Jun 25, 2024

i'm wondering: the improvement consists of not doing the CP scanning in the boot() method, but in the constructor, right? so it would only come into play when the runtime is initialized on every method, but not on every class, right?

the advantage kicks in also in the "runtime per class" case, e.g. in the "e2e transfer tests" where a single runtime is used multiple time across different tests (see backend-service runtime, used in 7 different test classes). Loading its classpath once will make things faster.

@ndr-brt ndr-brt merged commit e60d30b into eclipse-edc:main Jun 25, 2024
24 of 26 checks passed
@ndr-brt ndr-brt deleted the speed-up-e2e branch June 25, 2024 14:14
@paullatzelsperger
Copy link
Member

the advantage kicks in also in the "runtime per class" case, e.g. in the "e2e transfer tests" where a single runtime is used multiple time across different tests (see backend-service runtime, used in 7 different test classes).

true. seriously cool shit!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Anything related to the CI/CD Pipeline on Github Actions refactoring Cleaning up code and dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants