Skip to content

chore: decouple lint/test scripts, standardize naming, and create reu… - #204

Draft
matthieu-crouzet wants to merge 1 commit into
AmadeusITGroup:mainfrom
matthieu-crouzet:chore/decouple-lint-test-standardize-scripts
Draft

chore: decouple lint/test scripts, standardize naming, and create reu…#204
matthieu-crouzet wants to merge 1 commit into
AmadeusITGroup:mainfrom
matthieu-crouzet:chore/decouple-lint-test-standardize-scripts

Conversation

@matthieu-crouzet

@matthieu-crouzet matthieu-crouzet commented Mar 18, 2026

Copy link
Copy Markdown
Contributor

…sable CI action

- Remove lint from pretest to decouple linting from testing
- Rename compile → compile:extension, compile-tests → compile:tests, watch-tests → watch:tests
- Eliminate redundant compile-tests calls across npm scripts
- Remove unnecessary matrix from validate job, use env.NODE_VERSION
- Create .github/actions/setup-repo composite action (node setup + install)
- Externalize harden-runner and checkout from setup-repo so callers control them directly
- Use setup-repo action across all CI/publish workflows where applicable
- Update all docs, scripts, and CI workflows accordingly

Description

Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • 📝 Documentation update
  • ♻️ Code refactoring (no functional changes)
  • ⚡ Performance improvement
  • 🧪 Test coverage improvement
  • 🔧 Configuration/build changes

Related Issues

Closes #
Fixes #
Relates to #

Changes Made

Testing

Test Coverage

  • Unit tests added/updated
  • Integration tests added/updated
  • Manual testing completed
  • All existing tests pass

Manual Testing Steps

Tested On

  • macOS

  • Windows

  • Linux

  • VS Code Stable

  • VS Code Insiders

Screenshots

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings or errors
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published

Documentation

  • README.md updated
  • JSDoc comments added/updated
  • No documentation changes needed

Additional Notes

Reviewer Guidelines

Please pay special attention to:


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache License 2.0.

@matthieu-crouzet
matthieu-crouzet force-pushed the chore/decouple-lint-test-standardize-scripts branch 2 times, most recently from 03dd514 to 9f74867 Compare March 18, 2026 17:02
…sable CI action

- Remove lint from pretest to decouple linting from testing
- Rename compile → compile:extension, compile-tests → compile:tests, watch-tests → watch:tests
- Eliminate redundant compile-tests calls across npm scripts
- Remove unnecessary matrix from validate job, use env.NODE_VERSION
- Create .github/actions/setup-repo composite action (node setup + install)
- Externalize harden-runner and checkout from setup-repo so callers control them directly
- Use setup-repo action across all CI/publish workflows where applicable
- Update all docs, scripts, and CI workflows accordingly

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@matthieu-crouzet
matthieu-crouzet force-pushed the chore/decouple-lint-test-standardize-scripts branch from 9f74867 to 0deac84 Compare March 19, 2026 08:15

@gblanc-1a gblanc-1a left a comment

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.

Duplication on the first comment is for me the most important blocking command

Comment thread .github/actions/setup-repo/action.yml
Comment thread .github/workflows/lib-collection-scripts-ci.yml
Comment thread .github/workflows/vscode-extension-secure-ci.yml
Comment thread .github/workflows/vscode-extension-secure-ci.yml
Comment thread package.json
"test:unit": "npx mocha --ui tdd --require ./test/mocha.setup.js --require ./test/unit.setup.js 'test-dist/test/**/*.test.js' --ignore 'test-dist/test/suite/**/*.test.js' --timeout 5000",
"test:integration": "npm run compile-tests && node ./test/runExtensionTests.js",
"test:all": "npm run compile-tests && npm run test:unit && npm run test:integration",
"test:coverage": "npm run compile-tests && c8 npm run test:all",

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.

What do you think of a test:ai that does both compilation and test one after the other ?
To avoid having the ai trigger multiple commands / forgetting about compiling.

Happened to me already multiple times that the AI forgot to compile before running a direct test command was thinking the same might happen or we will need to update the AGENTS.md

What do you think ? Would even go further if we go with test:ai and add the rm -rf of the test-dist.
Happened regularly to me when switching branches to review PR

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.

I'm not in favor of test:ai
I didn't face that issue using AI, maybe because it's explained in the troubleshooting sectiong of the readme

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.

As agreed we should put back the test compilation until we change to a more modern testing framework that handle it by itself

Comment thread lib/package.json
"build": "tsc",
"watch": "tsc --watch",
"compile-tests": "tsc -p tsconfig.test.json",
"test": "npm run compile-tests && mocha 'dist-test/test/**/*.test.js'",

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.

As discussed we should put back the compile-tests in there to avoid having to do it manually each time we update a test

run: npm run build --workspace=lib

- name: Run linter
run: npm run lint

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.

Suggested change
run: npm run lint
run: npm run lint && npm run lint --workspace=lib

@matthieu-crouzet
matthieu-crouzet marked this pull request as draft March 23, 2026 08:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

2 participants