avajs / ava

Node.js test runner that lets you develop with confidence 🚀
MIT License
20.74k stars 1.41k forks source link

Parallel CI runs in GitLab CI break matrix #2994

Closed lenovouser closed 2 years ago

lenovouser commented 2 years ago

Issuehunt badges

When using a GitLab CI configuration like this:

test:
    image: node:$NODE_VERSION
    stage: test
    interruptible: true
    script:
        - npm run test
    parallel:
        matrix:
            - NODE_VERSION: [14, 16, lts, latest]

Ava assumes that the tests are being run in parallel, and thus splits the tests up into four. It then runs only a part of the tests in each node version, instead of testing everything on all node versions. This is the correct behavior when it is set like this:

test:
    parallel: 4

But not when using the matrix keyword, since that is used

to run a job multiple times in parallel in a single pipeline, but with different variable values for each instance of the job

(From docs.gitlab.com/ci/yaml)


IssueHunt Summary #### [il3ven il3ven](https://issuehunt.io/u/il3ven) has been rewarded. ### Backers (Total: $25.00) - [panascais panascais](https://issuehunt.io/u/panascais) ($25.00) ### Submitted pull Requests - [#3001 Allow parallel builds to be disabled](https://issuehunt.io/r/avajs/ava/pull/3001) --- ### Tips - Checkout the [Issuehunt explorer](https://issuehunt.io/r/avajs/ava/) to discover more funded issues. - Need some help from other developers? [Add your repositories](https://issuehunt.io/r/new) on IssueHunt to raise funds.
lenovouser commented 2 years ago

As far as I have seen I can't disable the automatic detection in Ava, nor circumvent this somehow in the GitLab CI configuration

novemberborn commented 2 years ago

@lenovouser you're right, that's not the intention!

Do you know which environment variables are set by GitLab? We rely on https://www.npmjs.com/package/ci-parallel-vars — perhaps that needs a patch to better handle this scenario?

If that isn't possible we can look at an option to disable this behavior.

lenovouser commented 2 years ago

@novemberborn GitLab is setting CI_NODE_INDEX and CI_NODE_TOTAL in either case of parallel or matrix.

There doesn't seem to be the possibility of detecting matrix according to docs.gitlab.com/ci/variables/predefined, which makes it unfixable in npmjs.com/package/ci-parallel-vars unless I missed something.

I think adding a option to disable that behavior is the best path forward.

issuehunt-oss[bot] commented 2 years ago

@panascais has funded $25.00 to this issue.


novemberborn commented 2 years ago

Thanks @lenovouser, I agree.

I think this should be a configuration option only (not a CLI flag). What do you think of utilizeParallelBuilds or utilizeParallelMachines? It would default to true, but can be set to false.

This condition can then check for conf.utilizeParallelBuilds !== false: https://github.com/avajs/ava/blob/ae0042c95d2dbe72b4910931232ea15f6071e04e/lib/cli.js#L396-L399

I don't believe we have test coverage for this but I haven't checked. It's a bit tricky to test I think, so maybe it's OK.

lenovouser commented 2 years ago

Great, I think utilizeParallelBuilds makes a bit more sense 👍🏻

issuehunt-oss[bot] commented 2 years ago

@sindresorhus has rewarded $22.50 to @il3ven. See it on IssueHunt