Use new config data when Infection has been executed at the first time

[sanmai]

For one, InfectionConfig should be using known default values, but not access the configuration file that is non-existing at the time of configuration. For another, JsonFile should not continue as usual even if it is given a non-existing file.

Fixes #576

Kudos to @sidz for his effort in #577 (E2E test there is not exactly correct).

[sidz]

fyi: I've rebased my PR as well.

[sanmai]

Builds are failing because of #631. Otherwise all seems to be fine.

[maks-rafalko]

Seems like the only one reason why $config variable is used here is to get default test framework name ($config->getTestFramework()) when infection.json does not exist.

And since config does not exist, it always defaults to phpunit.

infection/src/Config/InfectionConfig.php

Line 155 in 5b02e8e

return $this->config->testFramework ?? 'phpunit';

$this->config->testFramework is empty when infection.json does not exist. Because of

infection/src/Config/ConfigCreatorFacade.php

Lines 80 to 85 in 5b02e8e

	} catch (LocatorException $e) {
	// Generate an empty class to trigger `configure` command.
	$content = new \stdClass();
	$configLocation = getcwd();
	}

If we replace $config->getTestFramework() with TestFrameworkTypes::PHPUNIT, this will be the same PR as #577

I think we don't need $config instance here at all.

[sanmai]

I've seen this done in #577, but decided to keep the current behavior. For one because if configuration values are coming from one source, then you don't have to walk around all code changing them. Change them in one place and be done. For another we may have an idea to guess the value in the future. Then, again, everything will be as ready for this as it is.

I think some of these reasons are behind why this was done like that in the first place.

Do you still think we should stick to PHPUnit defaulting with a plain constant?

[maks-rafalko]

Apologize for the late reply.

What you say is perfectly valid. The only issue with this particular line of code is that it was my mistake (read "bug") that I've added it here. Let me explain:

This is a commit that adds this config here: 2859730

From git commit message:

• Use default test framework if neither CLI nor config has the value for it

So, all I wanted to do, is to use default test framework when it's impossible to get it neither from CLI option nor from the config.

Why was it a mistake? Because $config->getTestFramework() is intended to return a value from infection.json, not a default TF. It just has a fallback.

What I think would be the right solution:

instead of calling instance level method ->getTestFramework(), I would introduce a new static method InfectionConfig::getDefaultTestFramework() and call it here statically.

1. It would resolve the issue with config initialization, because of a statically called method
2. It would make code more expressive because what we want to do here is really getting a default TF

[sanmai]

OK, I got what you're thinking. This makes sense.

[sanmai]

I went with TestFrameworkTypes::PHPUNIT for now.

(We can make an easy switch to InfectionConfig::getDefaultTestFramework() when it'll do anything in particular other than just returning TestFrameworkTypes::PHPUNIT.)

[maks-rafalko]

@sanmai @sidz any thought on the latest comments?

[maks-rafalko]

Thank you @sanmai and @sidz