Fix public pull requests coverage and docs workflows comments

[lrlunin]

Split the build steps which trigger on pull_request from the steps which make reports in this (parent) repository. The steps which make comments now triggered by the build workflows via workflow_run.

Check my playground repository for and make your pull requests to test the action: https://github.com/lunin-inc/amazing-repo

[ckolbPTB]

Can we have this as a PR from a fork to ensure it does what we think it should do? Currently I do not see any reports anymore here - or am I missing something?

[fzimmermann89]

Leonid has a test repo set up

And yes, this does not work yet

[lrlunin]

Leonid has a test repo set up

And yes, this does not work yet

It does work. Sync your pull request. There was only an issue with set the Workflow to be failed. The Report of docs and coveragw worked as expected though.

To get this work here you need to have the following permissions set.

[schuenke]

@lrlunin Maybe use this PR to add yourself to the authors list in the pyproject.toml file

[schuenke]

Leonid has a test repo set up
And yes, this does not work yet

It does work. Sync your pull request. There was only an issue with set the Workflow to be failed. The Report of docs and coveragw worked as expected though.

To get this work here you need to have the following permissions set.

This is something we would need to change in the organization settings. It's not possible to enable it for a single repo within an organization. However, it should also work by granting the permission on the workflow level, no?

[fzimmermann89]

some minor things and questions

[fzimmermann89]

this was a quick fix because i was to lazy to try out something better,
do you want to check if
https://stackoverflow.com/questions/62045967/is-there-a-way-to-continue-on-error-while-still-getting-correct-feedback
would work here?

it would be nice if the pytest run step would be marked as failure, but the following steps still run (to create the comment)

[lrlunin]

Actually, it works exactly as you say. The report_pytest.yml runs even if the pytest.yml fails. The question - what should it report in the message if there is no such output of pytest?

So far, if there is any failures or errors in the pytest then the pytest.yml marked as failed but the report is still here.

[fzimmermann89]

The issue originally was, that if pytest failed to to an error in one of the tests, it still write out the XML fail - BUT the following steps did not run and the comment that something failed was never created. Thus, we added an "or true" to the pytest run, to always make the step succeed. and then later on check if the test failed to mark the pipeline as failed (to see it in the checks GUI in a PR).

So, if pytest run but has errors, there should be a comment. and there should be a red x next to the pytest job.

if no pytest XML is created at all, there should be a red x next to the job. ideally the comment would be "Pytest crashed", but this is not that important.

Do you think this can work?

[lrlunin]

This sounds really good. Yeah, this should work.

[fzimmermann89]

i understand you comment here:
wouldnt the downlaod of the artifact fail if the creation of docs failed?
or will it delete the existing old documentation and not upload something if the build workflow failed?

can you clarify?

[lrlunin]

Both of the new workflows report_docs.yml or report_pytest.yml are always invoked when the docs.yml or pytest.yml are completed independet from their state.

For the pytest.yml I explicitly say that the pytest.yml should be marked as failed if any of the tests fails. The pytest report message will be generated anyway. This makes sence and is an expected behaviour.

For the docs.yml there is no scenario when the docs.yml's fail makes sence. If the docs.yml workflow would fail - then, presumably, no html will be built and no artifact will be uploaded too. Then, obviosly, report_docs.yml will fail at the very first step.

I guess a little bit cleaner solution would be to check if the docs.yml is failed and then do not run the report_docs.yml at all and have it skipped. An error related to the no existence of empty artifact link will bring not much sense for the user.

[fzimmermann89]

or, even better, if docs failed, replace the docs comment by a "Documentation build failed" message?

otherwise, there is the link to a old documentation from a previous commit..

[lrlunin]

I believe it works since 7ad8955 as expected.

For pytest:

1. if the test case fails then is report comment generated, xml is parsed in the pytest step an marked this step as failed.
2. if the pytest command itself fails then this step is aborted (xml parse is not reached), upload artifacts is marked with always. the pytest step is marked as failed due to the pytest exit. the comment workflow gets the artifact anyway, tries to create a normal coverage report. if it is empty - post a comment that the pytest coverage is not available

For docs:

1. if the docu is built succesfully it will be uploaded as artifact and artifact url will be not empty. the report_docs checks if the artifact-url is empty or not.
2. if the docu is not built (sphinx-build command fails) then there is no Documentation uploaded articfact and the url would be empty. if the artifact-url is empty the report_docs post a comment that the documentation build failed.