Skip to content

Introduce --logger-text option to enforce text logger file path#2438

Merged
maks-rafalko merged 1 commit intoinfection:masterfrom
romm:feat/logger-text-option
Oct 10, 2025
Merged

Introduce --logger-text option to enforce text logger file path#2438
maks-rafalko merged 1 commit intoinfection:masterfrom
romm:feat/logger-text-option

Conversation

@romm
Copy link
Copy Markdown
Contributor

@romm romm commented Oct 10, 2025

Allows the usage of --logger-text option when running Infection, to override the "logs.text" option from the configuration file.

This PR:

I'm coming from https://github.com/phpstan/phpstan-doctrine/pull/686/files#r2416912180 — I like to work with a text log file when running Infection locally, however it makes sense to use php://stdout when running it in the pipeline. Adding this console option allows to override the option set in the configuration file.

I did not find an existing test for the --logger-html option, to create a similar test for this newly added option. What should I do?

If the PR is ok for you, I'll add a PR to update the documentation.

@maks-rafalko
Copy link
Copy Markdown
Member

maks-rafalko commented Oct 10, 2025

I did not find an existing test for the --logger-html option, to create a similar test for this newly added option. What should I do?

It's tested here I guess:

yield 'null html file log path with existing path from config file' => self::createValueForHtmlLogFilePath(
'/from-config.html',
null,
'/from-config.html',
);
yield 'absolute html file log path' => self::createValueForHtmlLogFilePath(
'/path/to/from-config.html',
null,
'/path/to/from-config.html',
);
yield 'relative html file log path' => self::createValueForHtmlLogFilePath(
'relative/path/to/from-config.html',
null,
'/path/to/relative/path/to/from-config.html',
);
yield 'override html file log path from CLI option with existing path from config file' => self::createValueForHtmlLogFilePath(
'/from-config.html',
'/from-cli.html',
'/from-cli.html',
);
yield 'set html file log path from CLI option when config file has no setting' => self::createValueForHtmlLogFilePath(
null,
'/from-cli.html',
'/from-cli.html',
);
yield 'null html file log path in config and CLI' => self::createValueForHtmlLogFilePath(
null,
null,
null,
);

You can create something similar. It's one of the hardest tests in Infection so sorry in advance 🤣

If the PR is ok for you, I'll add a PR to update the documentation.

absolutely makes sense

Regarding 100% MSI required here for PRs, feel free to skip this if it's too hard, we will merge anyway (I don't want to force you writing tests for the places covered badly previously).

@maks-rafalko maks-rafalko added DX Developer Experience Feature labels Oct 10, 2025
@romm romm force-pushed the feat/logger-text-option branch from a937eb5 to afd3b11 Compare October 10, 2025 09:55
@romm
Copy link
Copy Markdown
Contributor Author

romm commented Oct 10, 2025

Thanks for the hint! I shamefully copy-paster these tests, but that should be enough. 😊

@maks-rafalko
Copy link
Copy Markdown
Member

could you please fix https://github.com/infection/infection/actions/runs/18403064509/job/52436622828?pr=2438#step:7:48? And we are good to go

Allows the usage of `--logger-text` option when running Infection, to
override the "logs.text" option from the configuration file.
@romm romm force-pushed the feat/logger-text-option branch from afd3b11 to 1358dd2 Compare October 10, 2025 10:15
@maks-rafalko
Copy link
Copy Markdown
Member

Thank you @romm 👍

@maks-rafalko maks-rafalko merged commit f6fdf91 into infection:master Oct 10, 2025
55 of 57 checks passed
@maks-rafalko
Copy link
Copy Markdown
Member

@romm
Copy link
Copy Markdown
Contributor Author

romm commented Oct 10, 2025

That was fast! Thank you 😊

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

Labels

DX Developer Experience Feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants