fix: improve vmlens handling #1628
Conversation
Summary of ChangesHello @ThomasKrieger, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines the VMLens test setup by renaming a key test class and updating the Maven build configuration. These changes streamline the execution of VMLens tests, significantly reducing their runtime, and ensure that the VMLens plugin targets the correct set of tests. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the vmlens test setup to improve performance by isolating the concurrency tests. The main changes include renaming the test class to VmLensCT and configuring the vmlens-maven-plugin to selectively run only tests with the CT suffix. This is a good approach to speed up the build. The changes are logical and well-implemented. I have one minor suggestion regarding XML formatting in pom.xml to improve code consistency and maintainability.
| <includes> | ||
| <include>**/*CT.java</include> | ||
| </includes> |
There was a problem hiding this comment.
The indentation for the new <includes> block seems to be inconsistent, likely due to a mix of tabs and spaces. For better readability and maintainability, it's good practice to use consistent indentation. Adopting the 4-space indentation used elsewhere in this file would improve consistency.
| <includes> | |
| <include>**/*CT.java</include> | |
| </includes> | |
| <includes> | |
| <include>**/*CT.java</include> | |
| </includes> |
ThomasKrieger
changed the title
ThomasKrieger
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1628 +/- ##
============================================
+ Coverage 92.06% 93.41% +1.35%
- Complexity 488 491 +3
============================================
Files 46 46
Lines 1184 1184
Branches 105 105
============================================
+ Hits 1090 1106 +16
+ Misses 62 48 -14
+ Partials 32 30 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@ThomasKrieger the DCO check fails because the commits on this PR are not signed. Please sign them off and force push them, the check should succceed then |
ThomasKrieger
force-pushed
the
fix/improve-vmlens-handling
branch
from
57bbb37 to
0dd2c7f
Compare
VMLens test run now takes 4s and there is no risk of hanging tests. The hanging tests was during executing ArchUnitTests with VMLens. Renamed VmLensTest to VmLensCT and added a filter in the VMlens plugin. increased timeout for gherkin tests to 30s from 10s Signed-off-by: ThomasKrieger <[email protected]>
ThomasKrieger
force-pushed
the
fix/improve-vmlens-handling
branch
from
0dd2c7f to
0c64c82
Compare
and removed Awaitility.await() since it is not needed anymore since we wait during the setting of the states Signed-off-by: ThomasKrieger <[email protected]>
aepfli
left a comment
aepfli
left a comment
There was a problem hiding this comment.
Thank you! especially the neat way of waiting for the provider state - really cool
chrfwow
left a comment
chrfwow
left a comment
There was a problem hiding this comment.
Very good work! Thanks for the contribution
| case ERROR: | ||
| mockProvider.emitProviderReady(details); | ||
| mockProvider.emitProviderError(details); | ||
| mockProvider.emitProviderReady(details).await(); |
|
3 participants
This PR
Renamed the VmLensTest to VmLensCT and changed the VMLens plugin config to include only tests with postfix CT.
Related Issues
Fixes #1611
Notes
The VMLens test no takes something around 4s.
Follow-up Tasks
Please let me know if you prefer a special profile, or if the time is now o.k.