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

Add infection/infection to CI runs, when infection.json(.dist) exists #59

Merged

Conversation

Ocramius
Copy link
Member

@Ocramius Ocramius commented Sep 19, 2021

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

Description

Fixes #3

Adds vendor/bin/infection to executed CI pipeline matrix, when infection.json or infection.json.dist exist.

Note: I've not yet included roave/infection-static-analysis-plugin, which requires checking composer.json or vendor/bin/roave-infection-static-analysis-plugin (probably the former, since composer install happens inside the job, and not in the matrix)

I'm quite uncomfortable with this tooling not having tests/QA on its own, so I don't have a valid way to verify if my code does what it should do 🤔

@Ocramius Ocramius added this to the 1.11.0 milestone Sep 19, 2021
Ocramius added a commit to Ocramius/GeneratedHydrator that referenced this pull request Sep 19, 2021
src/create-jobs.js Outdated Show resolved Hide resolved
@boesing
Copy link
Member

boesing commented Sep 24, 2021

I'm quite uncomfortable with this tooling not having tests/QA on its own, so I don't have a valid way to verify if my code does what it should do 🤔

Usually, I do add composer.json to the root directory of this component containing those things you want to verify and then execute node index.js.

This will output the JSON and you can verify if it works as intended.

src/create-jobs.js Outdated Show resolved Hide resolved
@boesing
Copy link
Member

boesing commented Oct 5, 2021

@Ocramius I think I will start working on this on sunday as I want to kick out 1.11.0 as there are plenty of features I've added in the past weeks.

If you want to dive into this by yourself, just hit me up.

@boesing boesing force-pushed the feature/#3-include-infection-in-ci-runs branch from 0a52ebe to b9d6a6c Compare October 10, 2021 15:19
Signed-off-by: Maximilian Bösing <[email protected]>
@boesing
Copy link
Member

boesing commented Oct 10, 2021

@Ocramius would you mind double-checking the recent changes? If this suits your needs, I'd love to merge this PR

@boesing
Copy link
Member

boesing commented Oct 10, 2021

Okay, I'll summarize my changes:

I've moved the roave/infection-static-analysis-plugin into the same check which also verifies the existence of infection.json or infection.json.dist.

The rationale for this is that infection cannot work without these files and thus, we can merge the initially created checks (one for both) into one check.
image

So in the end, we still fulfill the initial requirements (preferring the roave plugin over infection itself in case it is installed).

@boesing boesing merged commit 7c2b382 into laminas:1.11.x Oct 10, 2021
@boesing boesing self-assigned this Oct 10, 2021
@boesing boesing linked an issue Oct 10, 2021 that may be closed by this pull request
@Ocramius Ocramius deleted the feature/#3-include-infection-in-ci-runs branch October 11, 2021 07:47
@Ocramius
Copy link
Member Author

@boesing interestingly, all my CI actions failed today :D

Reason: xdebug or pcov missing.

What's the best way to handle that, in your opinion? Should a require-dev addition be added? Should the CI setup install pcov when running infection?

@boesing
Copy link
Member

boesing commented Oct 11, 2021

As mentioned in slack: adding xdebug or pcov to require-dev or .laminas-ci.json would be fine.
But it seems that its fine to execute with phpdbg as you pointed out in #70 👍🏼

@Ocramius
Copy link
Member Author

I mostly preferred going with phpdbg because it is developed together with the PHP engine. Means 8.2 will have phpdbg out of the box :)

XDebug and pcov are fine, but there's always some wait time in between

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

Successfully merging this pull request may close these issues.

Include infection and roave/infection-static-analysis-plugin to matrix
2 participants