Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recognize opam-repo-ci #394

Closed
wants to merge 1 commit into from
Closed

Recognize opam-repo-ci #394

wants to merge 1 commit into from

Conversation

MisterDA
Copy link
Collaborator

@MisterDA MisterDA commented Sep 19, 2023

Recognize Alcotest is running in opam-repo-ci if the OPAM_REPO_CI env var is set to "true", and display all the tests (failures) by default.

@dinosaure
Copy link
Member

dinosaure commented Sep 19, 2023

I'm not really a fan of this PR since it implies that basically alcotest and opam-repo-ci are linked. Isn't there another way (on the opam-repo-ci side) to display the tests?

EDIT: I've just noticed the pattern-match on the different environments... Isn't there a way to merge all these environment variables so that GITHUB, ocaml-ci and opam-repo-ci use the same one to show the tests?

@MisterDA
Copy link
Collaborator Author

Well, they all define the CI env var to "true", so we can use just that.

There's the bonus that GitHub Actions parses the output log for workflow commands, such as

      - name: Create annotation for build error
        run: echo "::error file=app.js,line=1::Missing semicolon"

Alcotest recognizes that it's run in GHA and is able to emit these commands to better integrate in the GHA interface, by highlighting errors, or grouping test cases in drop down buttons. We could even have the GitHub interface link to the file where the error is raised, but that's a bit difficult to do considering the structure of Alcotest.

Neither OCaml-CI nor opam-repo-ci currently support such features.

@dinosaure
Copy link
Member

Hmmmhmm, I still have a doubt about the validity of having to change alcotest according to CI (and not vice versa) - as you mention, the CI environment variable would be sufficient for all CIs. What I have a problem with is the maintainance involved in such a design where the maintaineurs must always be synchronised with these CIs.

We could imagine extensions such as alcotest-github-actions or alcotest-ocaml-ci (to disociate the core release cycle from these). As it stands, I'm not going to ask you to do more than this PR but the question should remain open: I'm going to make an issue.

@yawaramin
Copy link

Don't all the CIs define the CI env var?

@MisterDA
Copy link
Collaborator Author

Closing, I'll switch to detecting CI only, since there's no special integration with opam-repo-ci.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants