Skip to content

Fix mutation test flow for branches/tags #118

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

Merged
merged 1 commit into from
Feb 25, 2021

Conversation

lcobucci
Copy link
Member

Q A
Documentation no
Bugfix no
BC Break no
New Feature no
RFC no
QA yes

Description

The variable github.base_ref isn't set when running the flow against branches/tags, which is leading the build to fail.

This creates conditional executions to make sure the build isn't accidentally broken any more.


Example of successful build on a branch (after this change): https://github.com/lcobucci/automatic-releases/runs/1982270612

The variable `github.base_ref` isn't set when running the flow against
branches/tags, which is leading the build to fail.

This creates conditional executions to make sure the build isn't
accidentally broken any more.

Signed-off-by: Luís Cobucci <[email protected]>
@lcobucci lcobucci requested a review from Ocramius February 25, 2021 20:09
Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

Still note that this workflow will go away when using https://github.com/laminas/laminas-ci-matrix-action

@Ocramius Ocramius added this to the 1.11.0 milestone Feb 25, 2021
@Ocramius Ocramius added the Bug Something isn't working label Feb 25, 2021
@Ocramius Ocramius self-assigned this Feb 25, 2021
@Ocramius Ocramius merged commit 368bcc2 into laminas:1.11.x Feb 25, 2021
@lcobucci
Copy link
Member Author

lcobucci commented Feb 25, 2021

LGTM

Still note that this workflow will go away when using https://github.com/laminas/laminas-ci-matrix-action

@Ocramius how should we distinguish between that tool and infection when building the matrix? It seems to be using the same configuration file, which is used to create the checks (as far as I understood from https://github.com/laminas/laminas-ci-matrix-container).

@lcobucci lcobucci deleted the only-run-mutation-diff-for-prs branch February 25, 2021 20:18
@Ocramius
Copy link
Member

how should we distinguish between that tool and infection when building the matrix?

Could you rephrase this please?

@lcobucci
Copy link
Member Author

Sure.

We match the configuration file of the tools to know whether or not to add them to the build: https://github.com/laminas/laminas-ci-matrix-container/blob/f6941918a63718d1c39c52062c4c61d3a1afd0f4/src/create-jobs.js#L79-L175

Infection-static-analysis-plugin uses the very same configuration file as Infection but using a different entry point to run the analysis.

My question is then: how to know when to add infection-static-analysis-plugin or infection to the build matrix?

@Ocramius
Copy link
Member

Ah, you are talking about roave/infection-static-analysis-plugin!

It should suffice to check if it is existing and executable in vendor/bin/, or part of require-dev, I suppose?

@lcobucci
Copy link
Member Author

Ah, you are talking about roave/infection-static-analysis-plugin!

Yeah, that's what we use here 🤣

It should suffice to check if it is existing and executable in vendor/bin/, or part of require-dev, I suppose?

I don't know... I'm not sure if the matrix calculation happens before or after the dependencies installation. We'd have to pick the approach which is less error-prone... I guess the former isn't an option since infection would also be there when roave/infection-static-analysis-plugin is installed, right?

Anyhow, I'll open an issue on the right repo and we can continue the discussion there 👍

@Ocramius
Copy link
Member

I don't know... I'm not sure if the matrix calculation happens before or after the dependencies installation. We'd have to pick the approach which is less error-prone... I guess the former isn't an option since infection would also be there when roave/infection-static-analysis-plugin is installed, right?

Agree, but it's our "best effort" for now.

Anyhow, I'll open an issue on the right repo and we can continue the discussion there 👍

Indeed, best way forward

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants