[RFC] Make infection easier to debug

[theofidry]

At its core I find infection quite hard to debug for a number of reasons, here is a non-exhaustive list

• We have quite a few debug/verbosity options:
  • --debug
  • --log-verbosity
  • --verbose
• It is not always clear what those debug/verbosity options are about and it's really frustrating to not being able to get the information you want after using one option and realising you missed another one
• When the temp folder is kept, you still need to look it up
• You have minimal information displayed to the user, e.g. of informations that could be displayed:
  • the configuration file used
  • if the server is going to be restarted
  • what code coverage generation tool is going to be used
  • where the coverage has been generated or with what command
  • what is being logged and where

I also aim to remove the need to require the configuration file in the future. For now those efforts are halted because of Configuation::getSrcDirs() used for Codeception (/cc @maks-rafalko). However even if resumed, it's imperative to have a way to know what was the config values. We already have a few "inferred" values, which are resolved between the config file and the command line options, and with a configuration-less config, there's only going to be a lot more of them. I think I could take from the approach I used with Box for that.

I would like to work on the different points above in the near future. I don't think there is much that is going to cause controversy in there, except for one main point which I would like to discuss now with you.

I think we should drop the idea of dumping temporary files in a temporary directory. Instead I would propose to dump everything in a .infection directory. The benefits are:

• the location is easy to know/see and pick up
• by picking the same location between two runs it's easier to ensure we're not leaving too many artefacts behind (they will always be in the same directory, so easier to clean up)
• it makes any artefact, if we desire to not delete them (e.g. in debug mode), easier to inspect and use as a consumer

A downside is that it requires users to add .infection to their .gitignore, but given that PHPUnit itself recently requires so with the cache file, I think it's an acceptable trade-off. WDYT?

[maks-rafalko]

Random thoughts

• can't you just use tmpDir config setting inside infection.json to achieve it?
• If we go this way, users should be able to opt-out and use another folder (currently possible with this tmpDir setting)
• there is an opinion that dot files/folders should be avoided, e.g. see arguments Language evolution overview proposal php/php-rfcs#2 (comment), so I'm not sure .infection the best name
• /tmp folder AFAIK is getting cleared automatically, which is a benefit since Infection produces a lot of files (hundrends of Megabytes or even Gigabytes). No automatic cleaning will be done with .infection

And the main (from my POV) point is

• several (many? all? some of them?) Unix systems use tmpfs for /tmp, so basically all the files are stored in RAM. It's for sure speeds the Infection process up, which is a good thing for our users.

[theofidry]

can't you just use tmpDir config setting inside infection.json to achieve it?

Yes totally, I was thinking of something along the lines of making it point to .infection by default

there is an opinion that dot files/folders should be avoided, e.g. see arguments php/php-rfcs#2 (comment), so I'm not sure .infection the best name

Not sure to understand the comment: dot files/directories are a common pattern. We already have .github, .composer and .ci in infection itself. PHPUnit .phpunit.result.cache, PHP-CS-Fixer .php_cs.{dist,cache}, git .git, .gitattributes... Dot files are not going anywhere. Happy to take a suggestion, but for a debug artefact this looks appropriate

/tmp folder AFAIK is getting cleared automatically, which is a benefit since Infection produces a lot of files (hundrends of Megabytes or even Gigabytes). No automatic cleaning will be done with .infection

I guess it gets cleared eventually yeah. But it's not immediate either, so in any case we need to clean up after a run.

My main beef here is UX: you have the logs giving you the commands, but can't use them easily because the files are gone.

But maybe this does not need to be systematic and could be done only with --debug: so that you don't have to fetch them from a weird tmp folder and can more easily inspect the content.

Also in case it wasn't clear: I don't mean we keep everything between two runs: a new run will wipe out the content of .infection that was previously left if existed.

several (many? all? some of them?) Unix systems use tmpfs for /tmp, so basically all the files are stored in RAM. It's for sure speeds the Infection process up, which is a good thing for our users.

But those tmp files are not meant to be re-used or inspected by an end-user like in our case. I'm not following the RAM issue though

[maks-rafalko]

My main beef here is UX: you have the logs giving you the commands, but can't use them easily because the files are gone.

command line is added to log file only with --debug, and files are not removed only with --debug. Isn't it a case?

I'm not following the RAM issue though

I'm not sure I got it. RAM is not an issue. Using /tmp which is attached to tmpfs is faster than using a disk, this is what I wanted to say.

Anyway, if users want to inspect the files, they need

• add --debug
• open the log file to understand where they are located
• open files

Having them in .infection won't make things easier - files still won't be stored there by default (they will be removed without --debug) and users will not know that .infection is the default folder.

Probably, as an idea, we can add a question to ConfigureCommand to ask people where they want to locate files, as well as providing info that files are stored only with --debug. We can even update .gitignore if the user chooses ./.infection in the same way as Symfony Flex does it in its recipes.

[sanmai]

Let's think you want Infection to make a report for users to copy-n-paste, in case, for example some unknown error happens, or some fairly common error like "class not found" when coverage files are out of sync.

And therefore you would want to accept and handle all exceptions on one place. If you think about, it'll make many difficulties we have with error handling and exceptions (it appears #1107 is there to work around them) much less of a problem. Say, now you have a CoverageDoesNotExistException, and it isn't just an exception, it's an exception with a help article attached to it. Why should it care? Is this a job for an exception to handle help articles?.. I'm not convinced. It isn't only exceptions. For example, PhpUnitXmlCoverageFactory now has to know which test framework we use, for no other reason than to include this information with an exception.

With all fairness, this is a valid approach when you want things done, but if you'd have every exception or error handled in one place, you can handle related tasks right here too. Say, what to show a user if a certain exception happened? You know it, and you have all the context, name of the framework, files, locations, etc. An exception object doesn't even need to care.

[maks-rafalko]

@sanmai I'm sorry I've read it 3 times and even thought it was posted to not correct issue

Could you please provide any examples supporting your message? I don't follow how this RFC solves what you described.

Aren't we discussing just moving log files from /tmp to .infection? If so, it's not related to all those exceptions and to #1107