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

[CMSP-833] add wpms fixture tests #119

Merged
merged 30 commits into from
Jan 23, 2024

Conversation

jazzsequence
Copy link
Contributor

@jazzsequence jazzsequence commented Jan 19, 2024

Overview

This pull request addresses CMSP-833 which unblocks #118 (CMSP-628). This is a fork from #118 and will merge into #118.

Q A
Bug fix? no
New feature? no
Has tests? yesno
BC breaks? no
Deprecations? no

Summary

  • Adds tests for the new pantheon-wpms-fixture
  • Adds a test configuration for testing WordPress multisite
  • Adds tests (really just copy pasta from the WP tests) for WP Multisite
  • Adds a new property to WpCliUpdate which looks to the update-tool configuration file for force-db-drop
    • This is needed because the PHPUnit tests run one after the other and the WPMS tests use the same database as the WP tests, so we need to drop the database before re-running the tests.
  • Adds a new helper method to the test Fixtures class to getPath

not sure if this will actually work, i guess we run it and find out...
@jazzsequence jazzsequence changed the base branch from master to CMSP-628-WPMS-Updates January 19, 2024 20:55
Copy link

guardrails bot commented Jan 22, 2024

All previously detected findings have been fixed. Good job! 👍🎉

We will keep this comment up-to-date as you go along and notify you of any security issues that we identify.


👉 Go to the dashboard for detailed results.

📥 Happy? Share your feedback with us.

@jazzsequence jazzsequence force-pushed the CMSP-833-add-wpms-fixture-tests branch from 38f1e4f to c57e0ad Compare January 23, 2024 16:06
@jazzsequence jazzsequence reopened this Jan 23, 2024
@jazzsequence jazzsequence changed the title Cmsp 833 add wpms fixture tests [CMSP-833] add wpms fixture tests Jan 23, 2024
@jazzsequence jazzsequence marked this pull request as ready for review January 23, 2024 17:19
@jazzsequence jazzsequence requested a review from a team as a code owner January 23, 2024 17:19
@jazzsequence
Copy link
Contributor Author

I think the CircleCI pipeline is still showing up in checks because we hadn't removed it at the point when @rwagner00 had forked his branch and my branch is a fork from that.

@jazzsequence jazzsequence merged commit 6d0d762 into CMSP-628-WPMS-Updates Jan 23, 2024
4 of 5 checks passed
@jazzsequence jazzsequence deleted the CMSP-833-add-wpms-fixture-tests branch January 23, 2024 17:29
@@ -83,7 +85,7 @@ public function findLatestVersion($major, $tag_prefix, $update_parameters)
*/
public function update(WorkingCopy $originalProject, array $parameters)
{
$forceDbDrop = !empty($parameters['force-db-drop']) ?? false;
$forceDbDrop = (!empty($parameters['force-db-drop']) || $this->forceDbDrop) ?? false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hate this. This does not need to be a clever one liner.

Comment on lines +181 to +186
// $path = $this->fixtures()->getPath('wpms');
// $wp_repo = $this->fixtures()->forkTestRepo('wpms');

// list all the files and directories at $path
// exec("set -e; echo 'dropping wp database via wp-cli'; wp db drop --yes --path=$path");
// exit(1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drop commented code.

Comment on lines +275 to +279
public function getPath($project)
{
return $this->getConfig()->get("projects.$project.path");
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only instance of this new function is commented out. Remove?

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.

3 participants