Track the requirements

[Eomm]

Prerequisites

• I have written a descriptive issue title
• I have searched existing issues to ensure the feature has not already been requested

🚀 Feature Proposal

After #8 we should document in a more clear way the requirements for the project to adopt the reusable workflow.

For example:

• npm test should not run the linter
• the lint script should run all the linters

Motivation

I think in future the requirements will grow up, so we should start listing it before it will be too late to understand how to set up a repository

Example

No response

[darkgl0w]

Totally agree with you.

I have a couple of "standard" npm scripts I use across my projects that improve the overall DX:

"ci:test": "npm run coverage && npm run test:types",
"coverage": "npm run unit -- --coverage-report=lcovonly",
"lint": "standard",
"lint:fix": "standard --fix",
"test": "npm run lint && npm run unit && npm run test:types",
"test:types": "tsd"
"unit": "tap -J \"test/**/*.test.js\"",
"unit:errors": "npm run unit -- -R terse",
"unit:report": "npm run unit -- --coverage-report=html",
"unit:verbose": "npm run unit -- -Rspec"

[Fdawgs]

Just to add, this was partially discussed in the fastify/plugins Discussions last year.

Throwing all of the tests into the test script would be ideal (as @darkgl0w's example shows), as this the standard name for test scripts, with a separate lint script for... linting.

[darkgl0w]

@Fdawgs something like this then ?

"coverage": "npm run unit -- --coverage-report=lcovonly", // useful to generate coverage report consumed by code coverage services
"lint": "standard",
"lint:fix": "standard --fix",
"test": "npm run coverage && npm run test:types", // target: ci run
"test:dev": "npm run lint && npm run unit && npm run test:types", // target: pre-commit run on user workflow (e.g.: my use case)
"test:types": "tsd"
"unit": "tap -J \"test/**/*.test.js\"", // just run the test suite
"unit:errors": "npm run unit -- -R terse", // run the test suite and display only errors (thanks to James Sumners that's now one of my best friends 😛)
"unit:report": "npm run unit -- --coverage-report=html", // useful to track the code coverage
"unit:verbose": "npm run unit -- -Rspec" // useful to track down tests failures and maintain a good DX (test descriptions, operations, plans ... etc)

[climba03003]

Just to add, this was partially discussed in the fastify/plugins Discussions last year.

Throwing all of the tests into the test script would be ideal (as @darkgl0w's example shows), as this the standard name for test scripts, with a separate lint script for... linting.

test script is used for pre-commit hooks. For test only, it should go for unit.

[darkgl0w]

test script is used for pre-commit hooks. For test only, it should go for unit.

This is how I use it on my workflow, I split each operations on dedicated scripts and in addition I have a CI dedicated script.

[darkgl0w]

update
we can solve this by allowing the customization of the script names to run (fallback to test).

https://github.com/darkgl0w/fastify-swagger/blob/exp-alt/.github/workflows/ci.yml
custom test script name CI run

[Fdawgs]

@fastify/plugins, if we can agree on what name the CI test script should have, as discussed in comments above, then this can get moving and can be applied to more plugin repos.

[darkgl0w]

In order of preference I would go for test:ci, ci:test or just ci so there will be no ambiguity about what is the script target.

PS: It would be awesome if we could agree on removing (at least) node 10 from the test matrix too.

[jsumners]

test:ci would be the correct name. And it should include -R terse.

[Eomm]

I think there are two topics:

1. the scripts needed for the workflow
2. the scripts needed by the developer

The first one requires:

• lint: to run once the lining as optimization
• test: run the test (usually lint+test+types). It is optimized for automation and reporting

I think we should not use test:ci because we are not talking about a complex application, but a simple node.js module. So, adding a dedicated script may lead to running out of sync the test and test:ci commands.

The latter is for us: it helps us to remember simple commands:

• lint:fix
• test:unit: runs just the test
• test:types

The other stuff adds too much noise into the scripts tag in my opinion.

[Fdawgs]

@Eomm I'd say this is done? We have the lint input for workflows to enable linting, and all package files in the repos have a lint and test besides the following:

• benchmarks has a test script but no lint script
• pacchetto has a lint script but no test script
• website has a lint script but no test script

for dir in */; do
    if [ -f "$dir/package.json" ]; then
        has_test=$(grep -q '"test":' "$dir/package.json" && echo "true" || echo "false")
        has_lint=$(grep -q '"lint":' "$dir/package.json" && echo "true" || echo "false")

        if [ "$has_test" == "false" ] && [ "$has_lint" == "true" ]; then
            echo "$dir has a test script but no lint script"
        fi

        if [ "$has_test" == "true" ] && [ "$has_lint" == "false" ]; then
            echo "$dir has a lint script but no test script"
        fi
    fi
done

These aren't plugins though, so can be ignored.