Separate linting from testing in npm scripts

[Sweetdevil144]

Which Caliper version are you using?

latest

Which Node.JS version are you using?

v21.7.2

Which operating system are you using?

MacOS arm64

Please provide some context for your error. For example, when did the error occur? What were you trying to achieve, and how?

I was working on setting up the testing environment for the Hyperledger Caliper project. I noticed an issue in the package.json file where the test script was set to run the linting command ("test": "npm run lint"). This occurred while I was reviewing the project setup and preparing to write and run tests.

My goal was to run the project's test suite using the npm test command, which is a standard practice in Node.js projects. However, due to the current script setup, running npm test only lints the code and does not run any tests.

I believe separating the linting and testing commands would be more beneficial and align with common practices, allowing developers to lint their code and run tests independently. This change would also make it clearer what each npm script is doing.

What was the observed incorrect behavior?

When I ran the npm test command, instead of executing the test suite as expected, it was running the linting command. This is not the typical behavior for a test script in a Node.js project.

Please provide the error logs and their surroundings.

npm run test

> @hyperledger/caliper-cli@0.5.1-unstable pretest
> npm run licchk

> @hyperledger/caliper-cli@0.5.1-unstable licchk
> license-check-and-add

Running using exclude exact_paths list
Running using exclude file type list
Trailing whitespace will be ignored in checking
No default format specified using {"prepend":"/*","append":"*/"} as backup
✔ All files have licenses.

> @hyperledger/caliper-cli@0.5.1-unstable test
> npm run lint

> @hyperledger/caliper-cli@0.5.1-unstable lint
> npx eslint .

(node:55792) [ESLINT_LEGACY_OBJECT_REST_SPREAD] DeprecationWarning: The 'parserOptions.ecmaFeatures.experimentalObjectRestSpread' option is deprecated. Use 'parserOptions.ecmaVersion' instead. (found in ".eslintrc.yml")
(Use `node --trace-deprecation ...` to show where the warning was created)

### Please provide your benchmark configuration file content, if possible.

_No response_

### Please provide your network configuration file content, if possible.

_No response_

### Please provide your workload module content, if possible.

```nodejs
"scripts": {
    "pretest": "npm run licchk",
    "licchk": "license-check-and-add",
    "test": "npm run lint",
    "lint": "npx eslint .",
    "nyc": "nyc --reporter=text --reporter=clover mocha --recursive -t 10000"
},

Please provide any additional information you deem relevant to the error.

This is not a typical setup for a Node.js project. Usually, the test script is used to run the project's test suite, and linting is handled by a separate script.

This setup could lead to confusion for developers who are accustomed to running npm test to execute the test suite. It could also potentially hinder the testing process, as developers might run npm test expecting to see test results, but instead, they would only see linting results.

[davidkel]

If you take a look at the top level package.json you will see

        "test": "npm run test --workspaces",

This runs all the tests in all the packages. If you look at caliper-core package you will see

    "test": "npm run lint && npm run nyc"

the npn run nyc runs the tests and also determines code coverage. So it's possible to run all the tests for the whole of caliper or individual package tests. The reason that you don't see this in caliper-cli and some of the other packages is because those packages don't have any tests so there is no point adding the entry to the test property. As caliper doesn't do any pre-processing or transpiling of the code, it make sense to include linting as part of the test stage to ensure that the code is actually ok before running the tests.

So there isn't anything wrong with the implementation as it is except for the fact that caliper is severely lacking in it's test suite and there is a mentorship project at hyperledger this year to address this so the hope is that this will advance the test suite and as tests are added to packages that have no tests then package.json 'test' property will be updated to reflect this.

[Sweetdevil144]

Maybe we can change the required script for linting to mpn run lint and add a command for test. That way, it will be much more clear and refracted. As goes for the LFX, I am applying too for the Caliper project. I'll prepare and submit a doc specifying the needs for the same.

[davidkel]

@Sweetdevil144 Sorry, not sure I agree with this. WIth a non compiled, non transpiled application such as caliper, it makes sense to combine linting with testing.No point running any tests if the linting fails. You can of course run npm run lint already and it will just do linting. Sorry but I don't see any value in changing what is already there.

[Sweetdevil144]

Yeah, Testing without Linting is not beneficial. I'll agree with it. Let's keep this issue in store here and later on discuss what needs to be done once our caliper repository is equipped with test-suites of its own and matches the requirements needed to do so.

[Sweetdevil144]

Hey @davidkel , I think this issue would be good if it was closed now. This resolves all confusion I had for the scripts.