test: add acceptance suite verifying unified test API equivalence#6846
test: add acceptance suite verifying unified test API equivalence#6846JamesPatrickGill wants to merge 1 commit into
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
This comment has been minimized.
This comment has been minimized.
james-snyk
force-pushed
the
test/acceptance-test-for-unified-scanner-test
branch
7 times, most recently
from
b0104d0 to
8e6701e
Compare
Adds equivalence harness that runs `snyk test` twice against the same fakeServer — once with the FF `internal_snyk_cli_use_unified_test_api_for_os_cli_test` off (legacy TS path → /test-dep-graph) and once with it on (os-flows path → /rest/orgs/:orgId/tests) — and asserts the dep graphs submitted, project counts, and exit codes match. Output presentation is intentionally not compared; only the structural data the backend sees. Starter corpus covers four fixtures (npm-package, maven-app, mono-repo-project --all-projects, no-supported-target-files). Helpers in equivalenceHelpers.ts are reusable for an extended sweep across sibling plugin/parser repos. # Conflicts: # cliv2-private/go.mod # cliv2-private/go.sum # cliv2/go.mod # cliv2/go.sum
james-snyk
force-pushed
the
test/acceptance-test-for-unified-scanner-test
branch
from
8e6701e to
7c89fa0
Compare
PR Reviewer Guide 🔍
|
There was a problem hiding this comment.
small suggestion: I think this ties a bit more to the OSS product line, so I might suggest we move somewhere under test/jest/acceptance/snyk-test. I think it's a bit more suitable than having this under the Jest utils which are command/product line agnostic.
| * FF off — TS CLI posts dep graphs to /test-dep-graph (legacy path) | ||
| * FF on — Go binary's os-flows extension posts to /rest/orgs/:orgId/tests | ||
| * | ||
| * The flag under test is `internal_snyk_cli_use_test_shim_for_os_cli_test` |
There was a problem hiding this comment.
question: I can't wrap my head around this: The ..._use_test_shim_... is the one we are looking to test, but the helpers use the ..._use_unified_test_api_.... Is this correct? Should this comment be adapted?
| })) | ||
| .sort((a, b) => a.displayTargetFile.localeCompare(b.displayTargetFile)); | ||
| } catch { | ||
| return []; |
There was a problem hiding this comment.
small nit: Maybe logging the error would be better to avoid silent failures.
|
|
||
| // Use a clean HOME for both runs so the Go binary's GAF reads from a fresh | ||
| // configstore with no locally-stored OAuth token that would override SNYK_API. | ||
| const cleanEnv = { ...env, HOME: os.tmpdir() }; |
There was a problem hiding this comment.
super small nit: I think the os.tmpdir() calls resolves the same location, so I'm not sure it's working as advertised by the comment above.
2 participants
Pull Request Submission Checklist
are release-note ready, emphasizing
what was changed, not how.
What does this PR do?
Add acceptance tests for the new unified scanner flow. This add the framework and the first few tests that compare the current test flow to the new test flow to make sure we return matching results.
To get behaviours to match needed to push a change to os-flows to make sure when the --all-project flag is enabled it handles errors in the same way.
What's the product update that needs to be communicated to CLI users?
None this is just adding tests.