Skip to content

chore: drive acceptance skips from CircleCI context (CLI-1464)#6801

Open
robertolopezlopez wants to merge 21 commits into
mainfrom
feat/CLI-1464-jest-ignore-paths
Open

chore: drive acceptance skips from CircleCI context (CLI-1464)#6801
robertolopezlopez wants to merge 21 commits into
mainfrom
feat/CLI-1464-jest-ignore-paths

Conversation

@robertolopezlopez
Copy link
Copy Markdown

@robertolopezlopez robertolopezlopez commented May 13, 2026

Pull Request Submission Checklist

  • Follows CONTRIBUTING guidelines
  • Commit messages
    are release-note ready, emphasizing
    what was changed, not how.
  • Includes detailed description of changes
  • Contains risk assessment (Low | Medium | High)
  • Highlights breaking API changes (if applicable)
  • Links to automated tests covering new functionality
  • Includes manual testing instructions (if necessary)
  • Updates relevant GitBook documentation (PR link: ___)
  • Includes product update to be announced in the next stable release notes

What does this PR do?

Where should the reviewer start?

How should this be manually tested?

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

@snyk-io
Copy link
Copy Markdown

snyk-io Bot commented May 13, 2026

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
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 13, 2026

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:

  • .eslintrc.json
  • test/jest/acceptance/snyk-code/snyk-code-user-journey.spec.ts
⚠️ There are multiple commits on your branch, please squash them locally before merging!
⚠️

"chore: revert TypeScript migration for createJestConfig and restore JavaScript configuration" is too long. Keep the first line of your commit message under 72 characters.

⚠️

"Revert "chore: refactor and modularize Jest ignore list and skip tests handling"" is too long. Keep the first line of your commit message under 72 characters.

⚠️

"chore: add missing dependencies to check-dependencies.config.ts for TypeScript and Jest" is too long. Keep the first line of your commit message under 72 characters.

⚠️

"chore: simplify destructuring in createJestConfig for testPathIgnorePatterns" is too long. Keep the first line of your commit message under 72 characters.

⚠️

"chore: migrate Jest configurations to TypeScript and reorganize createJestConfig" is too long. Keep the first line of your commit message under 72 characters.

⚠️

"chore: log skipped test IDs in acceptanceIt for improved test observability" is too long. Keep the first line of your commit message under 72 characters.

⚠️

"chore: remove TAP JUnit sanitation script and update observability logging in docs" is too long. Keep the first line of your commit message under 72 characters.

⚠️

"chore: migrate Jest configurations to TypeScript and enhance ignore fragment validation" is too long. Keep the first line of your commit message under 72 characters.

⚠️

"chore(circleci): simplify TAP test command by removing redundant error handling and cleanup steps" is too long. Keep the first line of your commit message under 72 characters.

⚠️

"chore(circleci): update yq installation path to $HOME/.local/bin and adjust PATH accordingly" is too long. Keep the first line of your commit message under 72 characters.

⚠️

"chore: add acceptanceIt to custom test block functions in ESLint config and remove redundant eslint-disable comments" is too long. Keep the first line of your commit message under 72 characters.

⚠️

"test: migrate createJestConfig tests to TypeScript and refine implementation" is too long. Keep the first line of your commit message under 72 characters.

Generated by 🚫 dangerJS against 92ddc81

robertolopezlopez and others added 2 commits May 13, 2026 12:52
Operators can narrow or skip flaky acceptance coverage from CircleCI without skip commits: whole-file patterns via TEST_SNYK_IGNORE_LIST merged into Jest path ignores, and granular it() skips via TEST_SNYK_SKIP_TEST_IDS with acceptanceIt() stable ids. CONTRIBUTING documents the workflow; acceptance-tests jobs attach team-cli-workflow-context so those variables are available in CI.

Co-authored-by: Cursor <cursoragent@cursor.com>
@robertolopezlopez robertolopezlopez force-pushed the feat/CLI-1464-jest-ignore-paths branch from b5e2f0f to e6fc6b2 Compare May 13, 2026 10:56
@robertolopezlopez robertolopezlopez changed the title CLI-1464 ignore flaky acceptance tests from CircleCI context chore: drive acceptance skips from CircleCI context (CLI-1464) May 13, 2026
…nfig and remove redundant eslint-disable comments
Introduce a script to strip `<properties>` blocks from TAP's JUnit output, resolving parsing issues in CircleCI. Update `.circleci/config.yml` to invoke this script post-tests if output exists.
@robertolopezlopez robertolopezlopez marked this pull request as ready for review May 13, 2026 13:59
@robertolopezlopez robertolopezlopez requested review from a team as code owners May 13, 2026 13:59
@snyk-pr-review-bot

This comment has been minimized.

@snyk-pr-review-bot

This comment has been minimized.

@snyk-pr-review-bot

This comment has been minimized.

…ragment validation

Refactored `createJestConfig` and associated project-level Jest configs to TypeScript. Improved handling of environment-based ignore fragments by validating RegExp sources, skipping invalid entries, and logging detailed warnings for observability. Updated tests and documentation to reflect changes.
@snyk-pr-review-bot

This comment has been minimized.

@snyk-pr-review-bot

This comment has been minimized.

Comment thread test/jest/util/acceptanceTestSkipById.ts
@snyk-pr-review-bot

This comment has been minimized.

Comment thread test/jest/util/acceptanceTestSkipById.ts
Comment on lines +34 to +38
const ids = raw
.split(',')
.map((s) => s.trim())
.filter(Boolean);
cachedSkipIds = new Set(ids);
Copy link
Copy Markdown
Contributor

@danskmt danskmt May 15, 2026

Choose a reason for hiding this comment

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

is it possible in this flow for raw to be null, undefined or something like that?

Comment thread test/jest/util/acceptanceTestSkipById.ts
Comment on lines +28 to +38
const raw = process.env.TEST_SNYK_SKIP_TEST_IDS;
if (typeof raw !== 'string' || raw.trim() === '') {
cachedSkipIds = new Set();
return cachedSkipIds;
}

const ids = raw
.split(',')
.map((s) => s.trim())
.filter(Boolean);
cachedSkipIds = new Set(ids);
Copy link
Copy Markdown
Contributor

@danskmt danskmt May 15, 2026

Choose a reason for hiding this comment

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

I think you should use parseSnykIgnoreFragments from test/createJestConfig.js

Comment thread test/createJestConfig.js Outdated
Comment on lines +49 to +50
const { valid: snykFragments, invalid: invalidIgnoreFragments } =
partitionIgnoreFragments(parsedFragments);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why renaming valid and invalid to these names snykFragments and invalidIgnoreFragments?
I'd suggest using a single name also in the function to be clear, and avoid renaming

Comment thread test/createJestConfig.ts Outdated
Comment on lines +73 to +77
const { testPathIgnorePatterns: configTestPathIgnores, ...restConfig } =
config;
const extraPathIgnores = Array.isArray(configTestPathIgnores)
? configTestPathIgnores
: [];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

no need to rename here either

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I simplified this, please let me know if this was what you intended

Extracted `getSkipTestList` and `getSkipTestIds` utilities for processing environment-based test ignore and skip lists into standalone modules. Updated `createJestConfig` and associated files to use these utilities for improved validation, modularity, and readability. Added unit tests for new utilities.
@snyk-pr-review-bot

This comment has been minimized.

…teJestConfig`

Converted project-level Jest configs and `createJestConfig` to TypeScript for consistency. Moved helper utilities like `getSkipTestList` for better modularity. No functional changes, improved type safety and maintainability.
@snyk-pr-review-bot

This comment has been minimized.

Updated `acceptanceIt` to log a banner once and detailed warnings for each skipped test ID individually, improving test observability and debugging.
@snyk-pr-review-bot

This comment has been minimized.

@snyk-pr-review-bot

This comment has been minimized.

…s handling"

This reverts commit e204326.

Revert "chore: migrate Jest configurations to TypeScript and reorganize `createJestConfig`"

This reverts commit f4681f0.

Revert "chore: enhance `acceptanceIt` logging for skipped test IDs"

This reverts commit e6095ef.

Revert "chore: simplify destructuring in `createJestConfig` for `testPathIgnorePatterns`"

This reverts commit 36a5dd0.

Revert "chore: add missing dependencies to `check-dependencies.config.ts` for TypeScript and Jest"

This reverts commit 3b209b7.
@snyk-pr-review-bot

This comment has been minimized.

… JavaScript configuration

Reverts `createJestConfig` TypeScript implementation back to JavaScript for compatibility. Updates `ts-node` usage and module registration for scoped transpilation of test utilities.
@snyk-pr-review-bot
Copy link
Copy Markdown

PR Reviewer Guide 🔍

🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Broken Dependency 🔴 [critical]

The file requires ./jest/skip-test-list/getSkipTestList and registers ts-node for that directory, but the required module is not included in the PR diff. This will cause a MODULE_NOT_FOUND error when Jest attempts to load the configuration, preventing any tests from running.

const { getSkipTestList } = require('./jest/skip-test-list/getSkipTestList');
Operational Risk 🟠 [major]

Replacing hardcoded it.skip calls with acceptanceIt enables these tests by default. As noted in the existing comments, these tests were skipped due to 'persistent CI failures'. If the CircleCI context team-cli-workflow-context is not already populated with these specific skip IDs, this PR will cause immediate pipeline failures upon merging.

acceptanceIt(
  SnykCodeUserJourneyContextSkipIds.GOLANG_NATIVE_IGNORED_ISSUES_SEVERITY_THRESHOLD,
)('with --severity-threshold', async () => {
📚 Repository Context Analyzed

This review considered 24 relevant code sections from 14 files (average relevance: 0.61)

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.

2 participants