-
Notifications
You must be signed in to change notification settings - Fork 4
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
Enable scan tests #412
Merged
Merged
Enable scan tests #412
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mirekdlugosz
force-pushed
the
enable-scans
branch
from
June 5, 2023 11:53
63fb596
to
b961183
Compare
If we have object name, but not id (meaning - it was created through CLI or UI), and we fail to establish object id, don't try to delete this object - just assume it's already gone and move on.
It's hard to know exactly when autouse fixture will run. This one is especially problematic, as it may exit early depending on tests selected for execution and their order. Scanjob tests were ported to DataProvider, which can handle their dependencies. Report tests have their own fixture for scans.
Not only it's hard to know when exactly autouse fixture will run, this one also goes into DataProvider area of responsibility.
mirekdlugosz
force-pushed
the
enable-scans
branch
from
June 5, 2023 12:00
b961183
to
438dd90
Compare
Codecov Report
@@ Coverage Diff @@
## main #412 +/- ##
==========================================
- Coverage 81.22% 79.11% -2.11%
==========================================
Files 5 5
Lines 245 249 +4
==========================================
- Hits 199 197 -2
- Misses 46 52 +6
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
ruda
approved these changes
Jun 5, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is massive. Did an overall review, seems that everything is sound. LGTM.
infinitewarp
approved these changes
Jun 5, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR enables reports and scan / scanjobs tests. I tried to make commits relatively complete, so it's best to review things commit-by-commit.
It depends on config changes introduced in discovery-ci PR #7.
Most changes revolve around using DataProvider and untangling dependencies between functions and fixtures. I decided to not port tests that are marked as skipped - I ran them before starting this work, and it seems that referenced issues are still not solved.
Performance of current version is not great, as most tests run scans they need - this leads to servers being scanned multiple times. I think this is something we can try to tackle later on.
Test results are below. There's one failure, but that tests seems flaky - it usually fails, but I saw it passing few times. Another thing to tackle later.