Problem/Motivation

PrepareFilesystem does three things:

It sets up the file system to be appropriate for a Drupal installation and subsequently use run-tests.sh.

It starts up the phantomjs process.

It prints some status info about the PHP version.

Starting phantomjs and showing the PHP version belong in the environment phase. Setting up the filesystem for the system under test belong in the assessment phase.

PHP version reporting duties will be handled in #2843549: RunContainers should output php -i as an artifact

Proposed resolution

Split out these behaviors appropriately.

Allow preparing the filesystem to break the build if the filesystem couldn't be prepared.

Remaining tasks

Deal with PHP version reporting in #2843549: RunContainers should output php -i as an artifact

User interface changes

API changes

Data model changes

Original report:

Its all setup for the simpletest plugin, so it should happen when we run that plugin. Definitely doesnt need its own plugin as its behavior specific to simpletest. Besides, we need to do some differently things for d7 so we should keep it altogether to make it overrideable.

Comments

Mixologic created an issue. See original summary.

mile23’s picture

+1. PrepareFilesystem assumes that the containers are available anyway, so it shouldn't be in environment:

It looks like PrepareFilesystem doesn't even use FileHandlerTrait.

There's also this commented code that should end up uncommented:

//    if ($result !== 0) {
//      // Directory setup failed threw an error.
//      $this->io->drupalCIError("Prepare Filesystem failed", "Setting up the filesystem failed:  Error Code: $result");
//      throw new BuildTaskException("Setting up the filesystem failed:  Error Code: $result");
//    }
    //return $result->getSignal();
mile23’s picture

Title: Move Contents of Prepare Filesystem into Simpletest Plugin » Reorganize PrepareFilesystem into specific plugins
Category: Feature request » Task
Issue summary: View changes
mile23’s picture

Title: Reorganize PrepareFilesystem into specific plugins » Move Contents of Prepare Filesystem into Simpletest Plugin
Issue summary: View changes

Simplifying the scope back to what it was. :-)

Chatting in IRC we decided the filesystem set up should be encapsulated in the simpletest plugin.

We thought SimpletestD7 might just override the prepareFilesystem method, but it requires the setup, as well.

Starting phantomjs and php -v are still present in prepare_filesystem, which shouldn't really be called prepare_filesystem any more. The PHP version part will get moved away in #2843549: RunContainers should output php -i as an artifact. We can deal with where to put the phantomjs startup in another follow-up.

  • Mile23 committed 0c038e5 on 2843673-refactor-preparefilesystem
    Issue #2843673: Moved file prep parts of PrepareFilesystem to Simpletest...
mile23’s picture

Status: Active » Needs review

  • Mile23 committed 3896edb on 2843673-refactor-preparefilesystem
    Issue #2843673: Fixed incorrect docblock
    

  • Mixologic committed 156ecb3 on 2843673-refactor-preparefilesystem
    Issue #2843673: Renames prepare filesystem to start_phantomjs
    
  • Mixologic committed 5b74615 on 2843673-refactor-preparefilesystem
    Issue #2843673: updates yml files to no longer use prepare filesystem
    
  • Mixologic committed b05d3fc on 2843673-refactor-preparefilesystem
    Issue #2843673: removes extra yml and comments
    
Mixologic’s picture

Status: Needs review » Fixed

This got a shuffling, and is deployed.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.