[frontend] fix(atomic-testing): prevent detection/prevention results leaking across tests sharing the same endpoint (#5080)#5453
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5453 +/- ##
=========================================
Coverage 58.65% 58.65%
Complexity 4876 4876
=========================================
Files 1031 1031
Lines 30481 30481
Branches 2267 2267
=========================================
Hits 17879 17879
Misses 11554 11554
Partials 1048 1048 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Seb-MIGUEL
changed the title
Seb-MIGUEL
force-pushed
the
copilot/fix-detection-results-issue
branch
from
a12dfd6 to
4bb7c1a
Compare
|
Thank you for your contribution. This PR is but one step away from being ready for merging: all commits must be PGP-signed. To get started, please see https://docs.github.com/en/authentication/managing-commit-signature-verification/signing-commits |
…ns to prevent cross-test result pollution Agent-Logs-Url: https://github.com/OpenAEV-Platform/openaev/sessions/b92ea6e7-35ac-46f5-bb88-f735d8e20b00 Co-authored-by: Seb-MIGUEL <203619865+Seb-MIGUEL@users.noreply.github.com>
Agent-Logs-Url: https://github.com/OpenAEV-Platform/openaev/sessions/820c827e-404d-4733-802c-dc08a72b6345 Co-authored-by: Seb-MIGUEL <203619865+Seb-MIGUEL@users.noreply.github.com>
Seb-MIGUEL
force-pushed
the
copilot/fix-detection-results-issue
branch
from
4bb7c1a to
1918171
Compare
antoinemzs
left a comment
antoinemzs
left a comment
There was a problem hiding this comment.
The proposal is that we always fetch fresh data from the backend based on the triple injectId | targetId | expectationType, which seems a rather blunt solution to the underlying problem.
The mistake in the original code was that the store was queried for all expectations based on the tuple targetId | expectationType which seems to only be missing the injectId filter.
The original code (deleted in the proposal):
getInjectExpectationsByAsset: (id, type) => entities('injectexpectations', state).filter(
i => (i.get('inject_expectation_asset') === id && i.get('inject_expectation_type') === type),
),IMO the correct solution would still leverage the store but simply introduce an inject filter to the store query.
| export const fetchTargetResultAssetWithAgents = (injectId: string, targetId: string, expectationType: string) => { | ||
| const uri = `${ATOMIC_TESTING_URI}/${injectId}/target_results/${targetId}/asset_with_agents?expectationType=${expectationType}`; | ||
| return getReferential(schema.arrayOfInjectexpectations, uri)(dispatch); | ||
| return simpleCall(uri); |
There was a problem hiding this comment.
So now we completely bypass the store to always get fresh data from the backend, we probably don't want that. @guillaumejparis @corinnekrych thoughts?
|
will address it in #5634 |
InjectTargetSearchTest.orFilterGroupCombinationShouldNotReturnAgentThatIsNotTargetOfInjectfails due to non-deterministic result orderingOption.IGNORING_ARRAY_ORDER(consistent with existing pattern in the same file)