Skip to content

Commit

Permalink
Add Port To WP_TESTS_DOMAIN (#49883)
Browse files Browse the repository at this point in the history
* Added Port Addition/Replacement Function

This new function will allow us to add/replace ports in
certain `wp-config.php` values.

* Refactored WP_SITEURL and WP_HOME Validation

* Always Use `WP_SITEURL` When Configuring WordPress

Since `WP_SITEURL` already contains the port in both the
development and test environments, there is no reason to
try and construct the URL with the port. Additionally, it is
more accurate to use the test environment's `WP_SITEURL`
since that's what WordPress will expect due to the constant.

* Automatically Add Port To `WP_TESTS_DOMAIN`

Tests should use the `WP_TESTS_DOMAIN` constant to
construct URLs in tests. This poses a problem, however,
because the port will be part of the URL in test environments.
In my review of WordPress Core and WooCommerce, adding
the port to `WP_TESTS_DOMAIN` should not break anything
and will make the container more resilient to port customizations.

* Removed Hardcoded `localhost` From PHPUnit Tests

To add resiliency to the test suite, we should use `WP_TESTS_DOMAIN`
instead of hardcoding the domain and port in the URLs.
  • Loading branch information
ObliviousHarmony authored Apr 20, 2023
1 parent 092039b commit b8733ad
Show file tree
Hide file tree
Showing 12 changed files with 232 additions and 94 deletions.
4 changes: 3 additions & 1 deletion packages/env/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@

## Unreleased

### Breaking Changes
### Breaking Change

- Use test environment's `WP_SITEURL` instead of `WP_TESTS_DOMAIN` as the WordPress URL.
- Automatically add the environment's port to `WP_TESTS_DOMAIN`.
- `run` command now has a `--env-cwd` option to set the working directory in the container for the command to execute from.

## 5.16.0 (2023-04-12)
Expand Down
4 changes: 2 additions & 2 deletions packages/env/lib/commands/start.js
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ module.exports = async function start( { spinner, debug, update, xdebug } ) {
}

const siteUrl = config.env.development.config.WP_SITEURL;
const e2eSiteUrl = `http://${ config.env.tests.config.WP_TESTS_DOMAIN }:${ config.env.tests.port }/`;
const testsSiteUrl = config.env.tests.config.WP_SITEURL;

const { out: mySQLAddress } = await dockerCompose.port(
'mysql',
Expand All @@ -213,7 +213,7 @@ module.exports = async function start( { spinner, debug, update, xdebug } ) {
.concat( siteUrl ? ` at ${ siteUrl }` : '.' )
.concat( '\n' )
.concat( 'WordPress test site started' )
.concat( e2eSiteUrl ? ` at ${ e2eSiteUrl }` : '.' )
.concat( testsSiteUrl ? ` at ${ testsSiteUrl }` : '.' )
.concat( '\n' )
.concat( `MySQL is listening on port ${ mySQLPort }` )
.concat(
Expand Down
32 changes: 32 additions & 0 deletions packages/env/lib/config/add-or-replace-port.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/**
* Internal dependencies
*/
const { ValidationError } = require( './validate-config' );

/**
* Adds or replaces the port to the given domain or URI.
*
* @param {string} input The domain or URI to operate on.
* @param {string} port The port to append.
* @param {boolean} [replace] Indicates whether or not the port should be replaced if one is already present. Defaults to true.
* @return {string} The string with the port added or replaced.
*/
module.exports = function addOrReplacePort( input, port, replace = true ) {
// This matches both domains and URIs with an optional port and anything
// that remains after. We can use this to build an output string that
// adds or replaces the port without making any other changes to the input.
const matches = input.match(
/^((?:.+:\/\/)?[a-z0-9.\-]+)(?::([0-9]+))?(.*)$/i
);
if ( ! matches ) {
throw new ValidationError( `Invalid domain or uri: ${ input }.` );
}

// When a port is already present we will do nothing if the caller doesn't want it to be replaced.
if ( matches[ 2 ] !== undefined && ! replace ) {
return input;
}

// Place the port in the correct location in the input.
return matches[ 1 ] + ':' + port + matches[ 3 ];
};
40 changes: 18 additions & 22 deletions packages/env/lib/config/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const { validateConfig, ValidationError } = require( './validate-config' );
const readRawConfigFile = require( './read-raw-config-file' );
const parseConfig = require( './parse-config' );
const { includeTestsPath, parseSourceString } = parseConfig;
const addOrReplacePort = require( './add-or-replace-port' );
const md5 = require( '../md5' );

/**
Expand Down Expand Up @@ -272,30 +273,25 @@ function withOverrides( config ) {
config.env.tests.phpVersion =
process.env.WP_ENV_PHP_VERSION || config.env.tests.phpVersion;

const updateEnvUrl = ( configKey ) => {
[ 'development', 'tests' ].forEach( ( envKey ) => {
try {
const baseUrl = new URL(
config.env[ envKey ].config[ configKey ]
);

// Don't overwrite the port of WP_HOME when set.
if ( ! ( configKey === 'WP_HOME' && !! baseUrl.port ) ) {
baseUrl.port = config.env[ envKey ].port;
}
// Some of our configuration options need to have the port added to them.
const addConfigPort = ( configKey ) => {
// Don't replace the port if one is set in WP_HOME.
const replace = configKey !== 'WP_HOME';

config.env[ envKey ].config[ configKey ] = baseUrl.toString();
} catch ( error ) {
throw new ValidationError(
`Invalid .wp-env.json: config.${ configKey } must be a valid URL.`
);
}
} );
config.env.development.config[ configKey ] = addOrReplacePort(
config.env.development.config[ configKey ],
config.env.development.port,
replace
);
config.env.tests.config[ configKey ] = addOrReplacePort(
config.env.tests.config[ configKey ],
config.env.tests.port,
replace
);
};

// Update wp config options to include the correct port number in the URL.
updateEnvUrl( 'WP_SITEURL' );
updateEnvUrl( 'WP_HOME' );
addConfigPort( 'WP_TESTS_DOMAIN' );
addConfigPort( 'WP_SITEURL' );
addConfigPort( 'WP_HOME' );

return config;
}
Expand Down
24 changes: 12 additions & 12 deletions packages/env/lib/config/test/__snapshots__/config.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ exports[`readConfig config file should match snapshot 1`] = `
"TEST_VAL3": false,
"WP_DEBUG": true,
"WP_ENVIRONMENT_TYPE": "local",
"WP_HOME": "http://localhost:2000/",
"WP_HOME": "http://localhost:2000",
"WP_PHP_BINARY": "php",
"WP_SITEURL": "http://localhost:2000/",
"WP_TESTS_DOMAIN": "localhost",
"WP_SITEURL": "http://localhost:2000",
"WP_TESTS_DOMAIN": "localhost:2000",
"WP_TESTS_EMAIL": "admin@example.org",
"WP_TESTS_TITLE": "Test Blog",
},
Expand All @@ -36,10 +36,10 @@ exports[`readConfig config file should match snapshot 1`] = `
"TEST_VAL3": false,
"WP_DEBUG": false,
"WP_ENVIRONMENT_TYPE": "local",
"WP_HOME": "http://localhost:1000/",
"WP_HOME": "http://localhost:1000",
"WP_PHP_BINARY": "php",
"WP_SITEURL": "http://localhost:1000/",
"WP_TESTS_DOMAIN": "localhost",
"WP_SITEURL": "http://localhost:1000",
"WP_TESTS_DOMAIN": "localhost:1000",
"WP_TESTS_EMAIL": "admin@example.org",
"WP_TESTS_TITLE": "Test Blog",
},
Expand All @@ -59,10 +59,10 @@ exports[`readConfig wp config values should use default config values 1`] = `
"SCRIPT_DEBUG": false,
"WP_DEBUG": false,
"WP_ENVIRONMENT_TYPE": "local",
"WP_HOME": "http://localhost:8889/",
"WP_HOME": "http://localhost:8889",
"WP_PHP_BINARY": "php",
"WP_SITEURL": "http://localhost:8889/",
"WP_TESTS_DOMAIN": "localhost",
"WP_SITEURL": "http://localhost:8889",
"WP_TESTS_DOMAIN": "localhost:8889",
"WP_TESTS_EMAIL": "admin@example.org",
"WP_TESTS_TITLE": "Test Blog",
}
Expand All @@ -73,10 +73,10 @@ exports[`readConfig wp config values should use default config values 2`] = `
"SCRIPT_DEBUG": true,
"WP_DEBUG": true,
"WP_ENVIRONMENT_TYPE": "local",
"WP_HOME": "http://localhost:8888/",
"WP_HOME": "http://localhost:8888",
"WP_PHP_BINARY": "php",
"WP_SITEURL": "http://localhost:8888/",
"WP_TESTS_DOMAIN": "localhost",
"WP_SITEURL": "http://localhost:8888",
"WP_TESTS_DOMAIN": "localhost:8888",
"WP_TESTS_EMAIL": "admin@example.org",
"WP_TESTS_TITLE": "Test Blog",
}
Expand Down
53 changes: 53 additions & 0 deletions packages/env/lib/config/test/add-or-replace-port.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/**
* Internal dependencies
*/
const addOrReplacePort = require( '../add-or-replace-port.js' );

describe( 'addOrReplacePort', () => {
beforeEach( () => {
jest.clearAllMocks();
} );

it( 'should add or replace port with various inputs', () => {
const testMap = [
// Addition
{ in: 'test', expect: 'test:101' },
{ in: 'test/test?test#test', expect: 'test:101/test?test#test' },
{ in: 'http://test.com', expect: 'http://test.com:101' },
{
in: 'http://test.com/test?test#test',
expect: 'http://test.com:101/test?test#test',
},
{ in: 'ssh://test.com', expect: 'ssh://test.com:101' },
{ in: 'test.com', expect: 'test.com:101' },

// Replacement
{ in: 'test:99', expect: 'test:101' },
{ in: 'test:99/test?test#test', expect: 'test:101/test?test#test' },
{ in: 'http://test.com:99', expect: 'http://test.com:101' },
{
in: 'http://test.com:99/test?test#test',
expect: 'http://test.com:101/test?test#test',
},
{ in: 'ssh://test.com:99', expect: 'ssh://test.com:101' },
{ in: 'test.com:99', expect: 'test.com:101' },
];

for ( const test of testMap ) {
const result = addOrReplacePort( test.in, '101' );
expect( result ).toEqual( test.expect );
}
} );

it( 'should do nothing if port is present but replacement is not requested', () => {
const testMap = [
{ in: 'test', expect: 'test:103' },
{ in: 'test:99', expect: 'test:99' },
];

for ( const test of testMap ) {
const result = addOrReplacePort( test.in, '103', false );
expect( result ).toEqual( test.expect );
}
} );
} );
76 changes: 57 additions & 19 deletions packages/env/lib/config/test/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,44 @@ describe( 'readConfig', () => {
}
} );

it( 'should throw a validation error if WP_SITEURL is not a valid URL', async () => {
readFile.mockImplementation( () =>
Promise.resolve(
JSON.stringify( {
config: {
WP_SITEURL: 'test',
},
} )
)
);
expect.assertions( 2 );
try {
await readConfig( '.wp-env.json' );
} catch ( error ) {
expect( error ).toBeInstanceOf( ValidationError );
expect( error.message ).toContain( 'must be a valid URL' );
}
} );

it( 'should throw a validation error if WP_HOME is not a valid URL', async () => {
readFile.mockImplementation( () =>
Promise.resolve(
JSON.stringify( {
config: {
WP_SITEURL: 'test',
},
} )
)
);
expect.assertions( 2 );
try {
await readConfig( '.wp-env.json' );
} catch ( error ) {
expect( error ).toBeInstanceOf( ValidationError );
expect( error.message ).toContain( 'must be a valid URL' );
}
} );

it( 'should infer a core config when ran from a core directory', async () => {
readFile.mockImplementation( () =>
Promise.reject( { code: 'ENOENT' } )
Expand Down Expand Up @@ -854,17 +892,17 @@ describe( 'readConfig', () => {
development: {
port: 1000,
config: {
WP_TESTS_DOMAIN: 'localhost',
WP_SITEURL: 'http://localhost:1000/',
WP_HOME: 'http://localhost:1000/',
WP_TESTS_DOMAIN: 'localhost:1000',
WP_SITEURL: 'http://localhost:1000',
WP_HOME: 'http://localhost:1000',
},
},
tests: {
port: 2000,
config: {
WP_TESTS_DOMAIN: 'localhost',
WP_SITEURL: 'http://localhost:2000/',
WP_HOME: 'http://localhost:2000/',
WP_TESTS_DOMAIN: 'localhost:2000',
WP_SITEURL: 'http://localhost:2000',
WP_HOME: 'http://localhost:2000',
},
},
},
Expand All @@ -878,7 +916,7 @@ describe( 'readConfig', () => {
port: 1000,
testsPort: 2000,
config: {
WP_HOME: 'http://localhost:3000/',
WP_HOME: 'http://localhost:3000',
},
} )
)
Expand All @@ -890,17 +928,17 @@ describe( 'readConfig', () => {
development: {
port: 1000,
config: {
WP_TESTS_DOMAIN: 'localhost',
WP_SITEURL: 'http://localhost:1000/',
WP_HOME: 'http://localhost:3000/',
WP_TESTS_DOMAIN: 'localhost:1000',
WP_SITEURL: 'http://localhost:1000',
WP_HOME: 'http://localhost:3000',
},
},
tests: {
port: 2000,
config: {
WP_TESTS_DOMAIN: 'localhost',
WP_SITEURL: 'http://localhost:2000/',
WP_HOME: 'http://localhost:3000/',
WP_TESTS_DOMAIN: 'localhost:2000',
WP_SITEURL: 'http://localhost:2000',
WP_HOME: 'http://localhost:3000',
},
},
},
Expand Down Expand Up @@ -1153,9 +1191,9 @@ describe( 'readConfig', () => {
WP_PHP_BINARY: 'php',
WP_TESTS_EMAIL: 'admin@example.org',
WP_TESTS_TITLE: 'Test Blog',
WP_TESTS_DOMAIN: 'localhost',
WP_SITEURL: 'http://localhost:8889/',
WP_HOME: 'http://localhost:8889/',
WP_TESTS_DOMAIN: 'localhost:8889',
WP_SITEURL: 'http://localhost:8889',
WP_HOME: 'http://localhost:8889',
} );

expect( config.env.development.config ).toEqual( {
Expand All @@ -1167,9 +1205,9 @@ describe( 'readConfig', () => {
WP_PHP_BINARY: 'php',
WP_TESTS_EMAIL: 'admin@example.org',
WP_TESTS_TITLE: 'Test Blog',
WP_TESTS_DOMAIN: 'localhost',
WP_SITEURL: 'http://localhost:8888/',
WP_HOME: 'http://localhost:8888/',
WP_TESTS_DOMAIN: 'localhost:8888',
WP_SITEURL: 'http://localhost:8888',
WP_HOME: 'http://localhost:8888',
} );
} );
} );
Expand Down
24 changes: 24 additions & 0 deletions packages/env/lib/config/validate-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,33 @@ function validateConfig( config, envLocation ) {
);
}

checkValidURL( envPrefix, config.config, 'WP_SITEURL' );
checkValidURL( envPrefix, config.config, 'WP_HOME' );

return config;
}

/**
* Validates the input and throws if it isn't a valid URL.
*
* @param {string} envPrefix The environment we're validating.
* @param {Object} config The configuration object we're looking at.
* @param {string} configKey The configuration key we're validating.
*/
function checkValidURL( envPrefix, config, configKey ) {
if ( config[ configKey ] === undefined ) {
return;
}

try {
new URL( config[ configKey ] );
} catch {
throw new ValidationError(
`Invalid .wp-env.json: "${ envPrefix }config.${ configKey }" must be a valid URL.`
);
}
}

module.exports = {
validateConfig,
ValidationError,
Expand Down
Loading

0 comments on commit b8733ad

Please sign in to comment.