Skip to content

docs: document that unit tests require TEST_SNYK_TOKEN - #7062

Merged
rrama merged 1 commit into
mainfrom
docs/unit-test-token-requirement
Aug 6, 2026
Merged

docs: document that unit tests require TEST_SNYK_TOKEN#7062
rrama merged 1 commit into
mainfrom
docs/unit-test-token-requirement

Conversation

@rrama

@rrama rrama commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Pull Request Submission Checklist

  • Follows CONTRIBUTING guidelines
  • Commit messages are release-note ready
  • Includes detailed description of changes
  • Contains risk assessment (Low)
  • Highlights breaking API changes (if applicable) — none
  • Links to automated tests covering new functionality — n/a, docs only
  • Includes manual testing instructions
  • Updates relevant GitBook documentation — n/a, contributor docs
  • Includes product update to be announced in the next stable release notes — n/a

What does this PR do?

Documents that npm run test:unit requires TEST_SNYK_TOKEN.

Without it, four suites fail with MissingApiTokenError, because they drive command entry points that call validateCredentials before doing anything else:

  • test/jest/unit/lib/commands/fix/fix.spec.ts
  • test/jest/unit/python/snyk-test-pyproject.spec.ts
  • test/jest/unit/pnpm/snyk-test-pnpm-project.spec.ts
  • test/jest/unit/snyk-code/snyk-code-test-report.spec.ts

This wasn't written down anywhere. CONTRIBUTING.md mentions TEST_SNYK_TOKEN only under acceptance tests, and AGENTS.md showed a bare npm run test:unit.

The part worth documenting explicitly is that reaching for SNYK_TOKEN — the obvious variable, and the one the CLI itself uses — has no effect. test/setup.js deletes SNYK_TOKEN and SNYK_API_KEY, then writes TEST_SNYK_TOKEN into the CLI user config. That's sensible (tests run against a known configuration rather than whatever the developer is authenticated as), but from the outside it looks like the token is being ignored and the tests are simply broken.

This came out of an agent reporting these suites as failures during environment setup, having concluded from the error that SNYK_TOKEN was needed. It isn't, and setting it changes nothing.

No behaviour is changed here — this only documents what already happens. Whether these suites should need a token is a separate question: CONTRIBUTING.md states unit tests "should not test services outside the code itself", so arguably they belong at the acceptance layer. Happy to raise that separately if the team agrees.

Where should the reviewer start?

CONTRIBUTING.md -> "Unit tests". The AGENTS.md change mirrors it.

How should this be manually tested?

npm run test:unit                            # 4 suites fail with MissingApiTokenError
TEST_SNYK_TOKEN=<token> npm run test:unit    # those 4 suites pass (22 tests)
SNYK_TOKEN=<token> npm run test:unit         # unchanged - still fails

What's the product update that needs to be communicated to CLI users?

None — contributor documentation only.

Risk assessment: Low

Documentation only.

Made with Cursor

@snyk-io

snyk-io Bot commented Jul 30, 2026

Copy link
Copy Markdown

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@github-actions

github-actions Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor
Warnings
⚠️

Since the CLI is unifying on a standard and improved tooling, we're starting to migrate old-style imports and exports to ES6 ones.
A file you've modified is using either module.exports or require(). If you can, please update them to ES6 import syntax and export syntax.
Files found:

  • AGENTS.md

Generated by 🚫 dangerJS against 4ef326e

@cursor
cursor Bot force-pushed the docs/unit-test-token-requirement branch from 5c5ee98 to 458b5a2 Compare July 31, 2026 17:08
@rrama
rrama force-pushed the docs/unit-test-token-requirement branch from 458b5a2 to e14c521 Compare July 31, 2026 17:11
Comment thread AGENTS.md
# TypeScript unit tests
npm run test:unit
# TypeScript unit tests (some suites validate credentials, so a token is required)
TEST_SNYK_TOKEN=<token> npm run test:unit

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docs do not instruct the agent that they need to build the CLI before they can run the tests. My agent didn't notice as it built the CLI first anyway, but would it be worth adding this requirement as well?

@rrama
rrama marked this pull request as ready for review August 3, 2026 10:08
@rrama
rrama requested a review from a team as a code owner August 3, 2026 10:08
@snyk-pr-review-bot

This comment has been minimized.

@rrama
rrama force-pushed the docs/unit-test-token-requirement branch from e14c521 to 9115b35 Compare August 6, 2026 13:14
@snyk-pr-review-bot

This comment has been minimized.

@rrama
rrama force-pushed the docs/unit-test-token-requirement branch from 9115b35 to 776f515 Compare August 6, 2026 14:36
Co-authored-by: Ben Durrans <Benjamin.Durrans@snyk.io>
@rrama
rrama force-pushed the docs/unit-test-token-requirement branch from 776f515 to 4ef326e Compare August 6, 2026 14:37
@snyk-pr-review-bot

This comment has been minimized.

@rrama
rrama enabled auto-merge August 6, 2026 14:38
@snyk-pr-review-bot

Copy link
Copy Markdown

PR Reviewer Guide 🔍

🧪 PR contains tests
🔒 No security concerns identified
⚡ No major issues detected
📚 Repository Context Analyzed

This review considered 5 relevant code sections from 2 files (average relevance: 0.44)

🤖 Repository instructions applied (from AGENTS.md)

@rrama
rrama merged commit 4cfa584 into main Aug 6, 2026
10 checks passed
@rrama
rrama deleted the docs/unit-test-token-requirement branch August 6, 2026 14:43

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verification results

Ran the verification skill against this diff. Changed files: AGENTS.md, CONTRIBUTING.md — both plain Markdown (mode 100644), no code changes → classified as a DOCS-class change, so only the adversarial reviewer lens ran.

Verdict: PASS — no findings.

The reviewer independently verified every factual claim in the new prose against the actual source it describes:

  • test/setup.js does strip SNYK_TOKEN/SNYK_API_KEY and write TEST_SNYK_TOKEN into the CLI user config, matching the new text in both files.
  • validateCredentials / apiTokenExists() does throw MissingApiTokenError when no token is configured, and is called unmocked in the command entry points the PR calls out.
  • The referenced suites and the pre-existing docs.snyk.io authentication link both check out.
  • No contradiction with the surrounding "unit tests shouldn't hit real services" guidance, since the check is local-only.

No Critical / Should Fix / Suggestion items were raised.

Auto-approval: this automation only auto-approves PRs that are pure dependency bumps (e.g. a go.mod/lockfile-only version change). Since this PR modifies documentation content rather than bumping a dependency, it is not being auto-approved, even though verification passed — it's left here for a human reviewer to approve.

Open in Web View Automation 

Sent by Cursor Automation: Automatic PR verification

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants