Shorten Node/Typescript test time

[michaelsauter]

Currently, the Node/Typescript build task tests take over 11 minutes:

--- PASS: TestTaskODSBuildTypescript (703.53s)
    --- PASS: TestTaskODSBuildTypescript/build_typescript_app_in_subdirectory_with_build_caching (143.10s)
    --- PASS: TestTaskODSBuildTypescript/fail_linting_typescript_app_and_generate_lint_report (19.06s)
    --- PASS: TestTaskODSBuildTypescript/fail_pulling_image_if_unsupported_node_version_is_specified (2.97s)
    --- PASS: TestTaskODSBuildTypescript/build_backend_typescript_app (183.43s)
    --- PASS: TestTaskODSBuildTypescript/build_typescript_app_with_custom_build_directory (119.73s)
    --- PASS: TestTaskODSBuildTypescript/build_typescript_app (111.68s)
    --- PASS: TestTaskODSBuildTypescript/build_typescript_app_in_subdirectory (122.10s)

For an example, see e.g. https://github.com/opendevstack/ods-pipeline/runs/6113417907?check_suite_focus=true. Compared to a total test run time of 29 minutes, this takes too much time compared to the rest.

Could we maybe cut a test? Or shorten one?

Two concrete ideas I have:

• use vanilla JS everywhere except in one test case
• turn of SQ scanning except in one test case

FYI @henninggross @netzartist @cschweikert

[cschweikert]

It would be interesting to see what exactly is taking so long within those builds. As far as I can see the logs are not showing timestamps in all the steps.

Anyways, I'm wondering, if running a linter by default is a good idea. When I got it correctly it is running this line

npx eslint src --ext "${LINT_FILE_EXT}" --format compact --max-warnings "${MAX_LINT_WARNINGS}" > eslint-report.txt

which first installs eslint and then does the linting against the code. Both things definetly take some time.

I see the following disadvantages with this behavior:

• eslint will be loaded and linting will be done no matter how much code is there and if linting would be of any value for a given codebase
• npx eslint ... loads the latest version of eslint and doesn't pin any specific version. This has implications on security and compatibility/test stability
• There might be situations where the code contains parts that fail the linting, e.g. generated code, third party code, etc. Those situations would require an option for turning off the linting.
• If linting is NOT practiced as part of the LOCAL development process, builds would fail more often.

Don't get me wrong. Techniques for increasing code quality like linting is an important practice. But eslint is not the only way of achieving a good code quality. Tools like prettier for example can do code formatting and autofixing most of the usual linting issues. And there is SonarQube as well which analyzes code quality beyond the "usual" linting.

BTW I have seen other places with npx and without version pinning (e.g. https://github.com/opendevstack/ods-pipeline/blob/master/test/testdata/workspaces/typescript-sample-app/package.json#L7)

tl;dr

My proposal would be to detect the existance of a .eslintrc.json file before running a linter. Or simply run npm run lint, where this task could pin the eslint version or do an echo 'no linting', if linting is not wanted 😉

[michaelsauter]

@cschweikert Excellent points!

And while this plays into test time, I think the topic of linting in the context of the Node/Typescript task warrants its own issue and discussion. How about @henninggross @netzartist @cschweikert (and maybe others interested?) you get together and discuss how to move forward? I think a design proposal / ADR explaining the decision and the tradeoffs would be a good fit here.

[henrjk]

@cschweikert see also #419 (comment) regarding showing timestamps.