src/tox.ini (coverage:run): Set concurrency = multiprocessing,threads

[mkoeppe]

• Partially addresses Concurrency bugs in codecov (coverage.py) workflow #36539

📝 Checklist

• The title is concise, informative, and self-explanatory.
• The description explains in detail what this PR is about.
• I have linked a relevant issue or discussion.
• I have created tests covering the changes.
• I have updated the documentation accordingly.

⌛ Dependencies

[github-actions]

Documentation preview for this PR (built with commit c1758ed; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

[kwankyu]

Let's run this again.

[mkoeppe]

Unknown concurrency choices: threads
Error: Process completed with exit code 1.

Hm...

[mkoeppe]

Thanks for the fix! Now it seems to work well

[mkoeppe]

There are still a few warnings Couldn't combine data file .coverage/.coverage.406549dc1e48.422.XQfVypyx: Data file '/sage/.coverage/.coverage.406549dc1e48.422.XQfVypyx' doesn't seem to be a coverage data file though.

[kwankyu]

Are we using "--parallel-mode" flag?

[kwankyu]

Ah, yes. I see "-p".

[kwankyu]

In build.yml, I see

      - name: Test all files (sage -t --long ${{ matrix.tests }})
        if: (success() || failure()) && steps.container.outcome == 'success'
        run: |
          mkdir .coverage
          rm -rf /sage/.coverage
          ln -s $(pwd)/.coverage /sage/
          cd /sage
          ./sage -python -m pip install coverage
          ./sage -python -m coverage run --rcfile=src/tox.ini src/bin/sage-runtests --force-lib --long -p4 --format github --random-seed=286735480429121101562228604801325644303 ${{ matrix.tests }} 

In src/tox.ini, I see

[testenv:coverage.py]
# https://coverage.readthedocs.io/en/latest/index.html
description =
    run the Sage doctester with Coverage.py
## This toxenv bypasses the virtual environment set up by tox.
passenv = {[sagedirect]passenv}
setenv  = {[sagedirect]setenv}
envdir  = {[sagedirect]envdir}
allowlist_externals = {[sagedirect]allowlist_externals}
commands_pre =
    {env:SAGE} -pip install -U coverage
commands =
    {env:SAGE} --python -m coverage run "{toxinidir}/../venv/bin/sage-runtests" -p 0 {posargs:--all}
commands_post =
    {env:SAGE} --python -m coverage combine
    {env:SAGE} --python -m coverage report   
...
[coverage:run]
source = sage
concurrency = multiprocessing
data_file = .coverage/.coverage
disable_warnings = no-data-collected  

There is "-p" in the command in tox.ini, but there is no "-p" in the command in build.yml. Is this flag applied to the coverage command in the workflow?

[mkoeppe]

There's a -p4 in the command

[kwankyu]

There's a -p4 in the command

isn't it for src/bin/sage-runtests? I meant -p for coverage command...

[kwankyu]

Now it seems to work well

I feel that we did not fix the root problem yet...

[mkoeppe]

There's a -p4 in the command

isn't it for src/bin/sage-runtests? I meant -p for coverage command...

All the -p flags are for sage-runtests, not for coverage

[kwankyu]

OK. Then the -p flag for coverage is

  -p, --parallel-mode   Append the machine name, process id and random number
                        to the data file name to simplify collecting data from
                        many processes.

Don't we need this flag? (That was perhaps what I was thinking a month ago)

[mkoeppe]

Don't we need this flag?

No, because that is configured in src/tox.ini already

[kwankyu]

Sorry but where in src/tox.ini?

[mkoeppe]

It's the very line that is modified here, in the [coverage:run] section

[kwankyu]

Do you mean concurrency = multiprocessing,thread implies --parallel-mode?

[kwankyu]

Now it seems to work well

I feel that we did not fix the root problem yet...

Do you agree with this?

[kwankyu]

Anyway, this PR is likely to alleviate the situation. Let's merge.

[kwankyu]

Let's keep the issue #36539 open though.

[mkoeppe]

I feel that we did not fix the root problem yet...

Do you agree with this?

Yes.

Thanks for the review!