Add RunnableTestCase to run fixed code in a test

[paslandau]

• added RunnableTestCase::assertOriginalAndFixedFileYieldSameResult($file)
  that takes a fixture file and expects fixture classes that implement the
  new RunnableInterface which exposes a run() method
• the fixture class is dynamically renamed to avoid naming conflicts
  and loaded via eval()
• fixtures that don't implement the RunnableInterface are ignored
  otherwise the run() method is called on the original class as well
  as on the fixed one and the output is expected to be equal
  (via assertSame()) by default
• the default assertion behavior can be changed via $assertion callable

[paslandau]

@TomasVotruba
Is there anything else apart from composer run complete-check that I can run locally to verify the code? Was kinda suprised to see the GH actions fail because everything looked fine locally :)

[TomasVotruba]

The composer command doesn't include all the actions.
Could you add missing scripts there?

[paslandau]

Could you add missing scripts there?

Had a quick look at the .github/workflows but I don't quite understand the setup and I don't feel comfortable changing the composer script without fully understanding the consequences.

Is this something that you can provide quickly on master?

Happy to fix any open issues in my PR as long as I get the corresponding feedback locally as the feedback loop via github actions only takes too long for that :/

[TomasVotruba]

It's usually the last run in each workflow.
E.g.

rector/.github/workflows/check_fixtures.yaml

Line 19 in 23b08fc

- run: composer check-fixtures

I might get into it in 1-2 hours and update the composer.json with full check

[TomasVotruba]

It seems most of issues are gone.

To apply Rector, just run:

composer rector-ci-fix

It's seems there is a bug in one old Rector rule though, I'll check it.

EDIT: fixed 17f9efc

[paslandau]

How long does a composer complete-check now take for you? Running all rector sets is pretty slow for me and actually just failed with

[...]
Set "safe07" is OK
Set "shopware55" is OK
Set "shopware56" is OK
Set "silverstripe" is OK

  [Symfony\Component\Process\Exception\ProcessTimedOutException]
  The process "php ci/run_all_sets.php" exceeded the timeout of 300 seconds.

(Running in Docker on a Windows 10 Host)

[TomasVotruba]

I never run it locally, so I don't know.

How could be the performance improved there?

[TomasVotruba]

This should be right under $className = $this->getTemporaryClassName(); with early reaturn,
as it can skipp all the Strings::replace()

[paslandau]

Don't think that would work.
is_a will only be able to recognize the class after eval has made it available to php

[TomasVotruba]

Ah, I didn't notice. Is there a way to handle this withou eval()?
That opens big black door.

[paslandau]

Well, to be fair you already have an unchecked require_once in \Rector\Core\Testing\PHPUnit\FixtureSplitter::splitContentToOriginalFileAndExpectedFile (which is basically the same) :)

Only way to work around this is to split the "orignal" file and the "fixed" files into separate files and also use different class names and proper autoloading. However, this clashes with the current way fixtures are written and checked (e.g. it won't be possible to do a full "equality" check of the contents any longer because the class names will be different - which I think is very helpful).

If you keep the "goal" in mind (i.e. "reuse existing test files and simply add an additional layer of validation), I'm afraid there is no way around using something that dymically injects code.

[paslandau]

@TomasVotruba
anything still open/unclear here?

[TomasVotruba]

Looks good me to me 👍