Skip to content

Conversation

@Pixel998
Copy link
Contributor

@Pixel998 Pixel998 commented Jan 18, 2026

Prerequisites checklist

What is the purpose of this pull request?

Standardizes the test and build orchestration across all packages in the monorepo.

What changes did you make? (Give an overview)

  • Introduce test:unit script across all packages for granular testing
  • Update test scripts to run both types and unit tests where applicable
  • Update CI workflow to use test:unit in the build matrix
  • Move package build logic to pretest scripts for consistency
  • Update package template to reflect new script standards

Related Issues

Is there anything you'd like reviewers to focus on?

@github-project-automation github-project-automation bot moved this to Needs Triage in Triage Jan 18, 2026
@fasttime fasttime moved this from Needs Triage to Triaging in Triage Jan 19, 2026
@fasttime
Copy link
Member

fasttime commented Jan 19, 2026

The test:types scripts are also run in CI at time when the whole project has been built, so I'm not sure If we'd want to provide separate scripts to run the type tests with and without a build first.

- name: Test types (core)
working-directory: packages/core
run: |
npm run test:types
- name: Test types (object-schema)
working-directory: packages/object-schema
run: |
npm run test:types
- name: Test types (config-array)
working-directory: packages/config-array
run: |
npm run test:types
- name: Test types (config-helpers)
working-directory: packages/config-helpers
run: |
npm run test:types
- name: Test types (plugin-kit)
working-directory: packages/plugin-kit
run: |
npm run test:types

Copy link
Member

@lumirlumir lumirlumir left a comment

Choose a reason for hiding this comment

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

I think this concerns the common convention across repositories that include a test:types script.

In the CSS repository, the tsc -p tests/types/tsconfig.json script appears to run after npm run build

(ref: https://github.com/eslint/css/blob/321d4705e396f0838e7960de45c8dd5947e2ecf1/package.json#L62).

However, aside from the CSS repository, I don't see other repositories running npm run build before testing types.

I don't have a strong preference, but it would be nice to standardize this across repositories.

@fasttime
Copy link
Member

fasttime commented Jan 19, 2026

In eslint/js, the type tests of each package are included when running the test script, and the test script builds the package through pretest if necessary. E.g. in espree:

    "pretest": "npm run build",
    "test": "npm run test:types && npm run test:cjs && npm run test:esm",
    "test:cjs": "mocha --color --reporter progress --timeout 30000 \"tests/**/*.test.cjs\"",
    "test:esm": "c8 mocha --color --reporter progress --timeout 30000 \"tests/**/*.test.js\"",
    "test:types": "tsd --typings dist/espree.d.ts"

https://github.com/eslint/js/blob/98caac0d92865a014220cf9e014df08f8913a4a4/packages/espree/package.json#L66-L70

If the intent is simplifying entering commands during development, perhaps this could be an option?

@lumirlumir
Copy link
Member

lumirlumir commented Jan 19, 2026

If the intent is simplifying entering commands during development, perhaps this could be an option?

I think it's a good option, since CSS and JSON also use the pretest lifecycle hook.

One more thing to discuss: unlike eslint/js, some repositories have commands under the test: scope such as test:coverage and test:jsr. Would it make sense to group those as well? Their nature differs somewhat from unit and type testing, so I'm not sure what the best approach is.

Maybe it would make sense to group only the type and unit tests into a single test script (excluding test:coverage and test:jsr) and add a pretest lifecycle hook?

To clarify, my suggestion is:

    "pretest": "npm run build",
    "test": "npm run test:types && npm run test:unit",
    "test:coverage": "c8 npm test",
    "test:jsr": "npx -y jsr@latest publish --dry-run",
    "test:types": "tsc -p tests/types/tsconfig.json"
    "test:unit": "mocha \"tests/**/*.test.js\" --timeout 30000",

@sethamus
Copy link
Contributor

I've hit this in our other repos too and it’s definitely a friction point—it's easy to waste time debugging a 'fail' that’s actually just a stale build.

I get the concern about CI redundancy, but maybe we could add the pretest hook for consistency and then a test:types:build script for local dev? That way we keep the CI lean but still have a one-step way to run standalone type tests without the whole suite.

@Pixel998
Copy link
Contributor Author

Maybe it would make sense to group only the type and unit tests into a single test script (excluding test:coverage and test:jsr) and add a pretest lifecycle hook?

Sounds good to me. @fasttime, what do you think about moving in this direction?

@fasttime
Copy link
Member

Maybe it would make sense to group only the type and unit tests into a single test script (excluding test:coverage and test:jsr) and add a pretest lifecycle hook?

Sounds good to me. @fasttime, what do you think about moving in this direction?

I think it's still good to run unit tests separately from type tests in GitHub Actions. Currently, this is done by running the top-level test script from the workflow file, which in turn runs npm test on each package:

run: |
npm install
npm test

"test": "npm test --workspaces --if-present",

If the test scripts in the packages are changed to include type tests, how will we run unit tests in CI? Shall we add a top-level test:unit script for that?

@Pixel998
Copy link
Contributor Author

If the test scripts in the packages are changed to include type tests, how will we run unit tests in CI? Shall we add a top-level test:unit script for that?

Sounds good 👍. We can add a top-level test:unit script:

"test:unit": "npm run test:unit --workspaces --if-present"

@fasttime
Copy link
Member

If the test scripts in the packages are changed to include type tests, how will we run unit tests in CI? Shall we add a top-level test:unit script for that?

Sounds good 👍. We can add a top-level test:unit script:

"test:unit": "npm run test:unit --workspaces --if-present"

That should work. Noting that npm test is also run in test:coverage scripts. To keep the current behavior there, those should be updated as well to run (I think) npm run build and then npm run test:unit.

@Pixel998 Pixel998 changed the title chore: run build before type tests build: standardize test scripts Jan 30, 2026
@fasttime fasttime moved this from Triaging to Implementing in Triage Jan 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Implementing

Development

Successfully merging this pull request may close these issues.

5 participants