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

tests/e2e - Isolate each call to civix. More representative of real usage #359

Merged
merged 1 commit into from
Jun 27, 2024

Conversation

totten
Copy link
Owner

@totten totten commented Jun 27, 2024

This is an off-shoot of work on #331. Here's the main comment that explains the purpose.


Ah, blerg. There's a test-interaction problem (i.e. tests pass individually but fail in bulk). E2E tests look like this:

   $this->civixGenerateModule(static::getKey(), [
      '--compatibility' => '5.69',
    ]);
    $this->civixGenerateUpgrader();
    $this->civixGenerateEntity('Bread');
    $this->civixGenerateEntity('Sandwich');
  • These steps actually run in the same PHP process. This is good for debugging/inspection -- but bad for fidelity. In real usage, they run as separate PHP processes.
  • If you run several tests in the same process, then the relevant files/functions only load once. (The first variant of function xyz() {...} stays in memory.)
  • This hasn't been a problem before because...
    • The code-generators were traditionally/typically additive (each test/step makes a new extension/file/function/class).
    • The code-generators were traditionally/typically passive (they don't necessarily boot CiviCRM; they just fiddle with strings).
  • But now...
    • It's starting to lean more on conditional construction (and regeneration) of *.civix.php. Steps aren't just additive -- they can change the content of *.civix.php. (This PR is a good example.)
    • It's starting to incorporate more active functionality from CiviCRM -- so some steps require booting (while other steps don't). (Increasingly, the files are actually loaded/executed.)
  • The overall effect is chaotic/painful to trace.
  • What's probably needed is to change the test-structure to improve isolation. We have a couple high-leverage points:
    • phpunit's --process-isolation puts each test-case in its own process. But it's not quite realistic -- individual steps would still share the same process.
    • CivixProjectTestTrait::civix() calls Symfony's CommandTester. Swap that with something similar that uses sub-processes. This could reveal some other hidden interactions, but it's probably better in the long run.

So this PR implements the second approach.

@totten totten merged commit 9374b6b into master Jun 27, 2024
1 check passed
@totten totten deleted the master-test branch June 27, 2024 22:05
@totten totten changed the title tests/e2e - Isolate each call to civix. More representative of real u… tests/e2e - Isolate each call to civix. More representative of real usage Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant