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

wp-now: Add tests that check resulting fs in different modes #339

Merged
merged 20 commits into from
May 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 0 additions & 14 deletions packages/wp-now/jest.config.ts

This file was deleted.

3 changes: 2 additions & 1 deletion packages/wp-now/project.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@
"outputs": ["coverage/packages/wp-now"],
"options": {
"passWithNoTests": true,
"reportsDirectory": "../../coverage/packages/wp-now/node"
"reportsDirectory": "../../coverage/packages/wp-now/node",
"testTimeout": 20000
}
}
},
Expand Down
9 changes: 5 additions & 4 deletions packages/wp-now/src/download.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
WP_NOW_PATH,
} from './constants';
import { isValidWordpressVersion } from './wp-playground-wordpress';
import { output } from './output';

function getWordPressVersionUrl(version = DEFAULT_WORDPRESS_VERSION) {
if (!isValidWordpressVersion(version)) {
Expand All @@ -37,7 +38,7 @@ async function downloadFileAndUnzip({
itemName,
}): Promise<DownloadFileAndUnzipResult> {
if (fs.existsSync(checkFinalPath)) {
console.log(`${itemName} folder already exists. Skipping download.`);
output?.log(`${itemName} folder already exists. Skipping download.`);
return { downloaded: false, statusCode: 0 };
}

Expand All @@ -46,7 +47,7 @@ async function downloadFileAndUnzip({
try {
fs.ensureDirSync(path.dirname(destinationFolder));

console.log(`Downloading ${itemName}...`);
output?.log(`Downloading ${itemName}...`);
const response = await new Promise<IncomingMessage>((resolve) =>
https.get(url, (response) => resolve(response))
);
Expand Down Expand Up @@ -79,7 +80,7 @@ async function downloadFileAndUnzip({
.promise();
return { downloaded: true, statusCode };
} catch (err) {
console.error(`Error downloading or unzipping ${itemName}:`, err);
output?.error(`Error downloading or unzipping ${itemName}:`, err);
}
return { downloaded: false, statusCode };
}
Expand All @@ -99,7 +100,7 @@ export async function downloadWordPress(
fs.ensureDirSync(path.dirname(finalFolder));
fs.renameSync(path.join(tempFolder, 'wordpress'), finalFolder);
} else if (404 === statusCode) {
console.log(
output?.log(
`WordPress ${wordPressVersion} not found. Check https://wordpress.org/download/releases/ for available versions.`
);
process.exit(1);
Expand Down
5 changes: 5 additions & 0 deletions packages/wp-now/src/output.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
function shouldOutput() {
return process.env.NODE_ENV !== 'test';
}

export const output = shouldOutput() ? console : null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I love it, thank you! Later on let's export a function to disable that manually for the VS Code extension

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sounds good, we may also disable the output for the PHP command we are adding in #242

9 changes: 5 additions & 4 deletions packages/wp-now/src/run-cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,16 @@ import { portFinder } from './port-finder';
import { DEFAULT_PHP_VERSION, DEFAULT_WORDPRESS_VERSION } from './constants';
import { SupportedPHPVersion } from '@php-wasm/universal';
import { spawn, SpawnOptionsWithoutStdio } from 'child_process';
import { output } from './output';

function startSpinner(message: string) {
process.stdout.write(`${message}...\n`);
return {
succeed: (text: string) => {
console.log(`${text}`);
output?.log(`${text}`);
},
fail: (text: string) => {
console.error(`${text}`);
output?.error(`${text}`);
},
};
}
Expand Down Expand Up @@ -61,7 +62,7 @@ export async function runCli() {
const { url } = await startServer(options);
openInDefaultBrowser(url);
} catch (error) {
console.error(error);
output?.error(error);
spinner.fail(
`Failed to start the server: ${
(error as Error).message
Expand Down Expand Up @@ -92,7 +93,7 @@ function openInDefaultBrowser(url: string) {
args = ['/c', `start ${url}`];
break;
default:
console.log(`Platform '${process.platform}' not supported`);
output?.log(`Platform '${process.platform}' not supported`);
return;
}
spawn(cmd, args);
Expand Down
5 changes: 3 additions & 2 deletions packages/wp-now/src/start-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import fileUpload from 'express-fileupload';
import { portFinder } from './port-finder';
import { NodePHP } from '@php-wasm/node';
import startWPNow from './wp-now';
import { output } from './output';

function requestBodyToMultipartFormData(json, boundary) {
let multipartData = '';
Expand Down Expand Up @@ -97,13 +98,13 @@ export async function startServer(
});
res.end(resp.bytes);
} catch (e) {
console.trace(e);
output?.trace(e);
}
});

const url = `http://127.0.0.1:${port}/`;
app.listen(port, () => {
console.log(`Server running at ${url}`);
output?.log(`Server running at ${url}`);
});

return {
Expand Down
Empty file.
Empty file.
177 changes: 175 additions & 2 deletions packages/wp-now/src/tests/wp-now.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
import { inferMode, parseOptions, WPNowMode, WPNowOptions } from '../wp-now';
import startWPNow, {
inferMode,
parseOptions,
WPNowMode,
WPNowOptions,
} from '../wp-now';
import fs from 'fs-extra';
import path from 'path';
import {
Expand All @@ -8,7 +13,8 @@ import {
isWordPressDirectory,
isWordPressDevelopDirectory,
} from '../wp-playground-wordpress';
import jest from 'jest-mock';
import os from 'os';
import crypto from 'crypto';

const exampleDir = __dirname + '/mode-examples';

Expand Down Expand Up @@ -144,3 +150,170 @@ test('isWordPressDevelopDirectory returns false for incomplete WordPress-develop
expect(isWordPressDevelopDirectory(projectPath)).toBe(false);
expect(inferMode(projectPath)).toBe(WPNowMode.INDEX);
});

describe('Test starting different modes', () => {
let tmpExampleDirectory;

/**
* Copy example directory to a temporary directory
*/
beforeEach(() => {
const tmpDirectory = os.tmpdir();
const directoryHash = crypto.randomBytes(20).toString('hex');
Copy link
Member

Choose a reason for hiding this comment

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

Let's add a wp-now-tests- prefix to the directoryHash so it's somewhat obvious where these directories are coming from.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good idea, I've added the flag. Note that we are removing the directory in afterEach, so ideally they shouldn't stay in /tmp after tests are executed. It is still a good change for cases in which the test crashes and doesn't run the cleanup method.


tmpExampleDirectory = path.join(
tmpDirectory,
`wp-now-tests-${directoryHash}`
);
fs.ensureDirSync(tmpExampleDirectory);
fs.copySync(exampleDir, tmpExampleDirectory);
});

/**
* Remove temporary directory
*/
afterEach(() => {
fs.rmSync(tmpExampleDirectory, { recursive: true, force: true });
});

/**
* Expect that all provided mount point paths are empty directories which are result of file system mounts.
*
* @param mountPaths List of mount point paths that should exist on file system.
* @param projectPath Project path.
*/
const expectEmptyMountPoints = (mountPaths, projectPath) => {
mountPaths.map((relativePath) => {
const fullPath = path.join(projectPath, relativePath);

expect(fs.existsSync(fullPath)).toBe(true);
expect(fs.readdirSync(fullPath)).toEqual([]);
expect(fs.lstatSync(fullPath).isDirectory()).toBe(true);
});
};

/**
* Expect that all listed files do not exist in project directory
*
* @param forbiddenFiles List of files that should not exist on file system.
* @param projectPath Project path.
*/
const expectForbiddenProjectFiles = (forbiddenFiles, projectPath) => {
forbiddenFiles.map((relativePath) => {
const fullPath = path.join(projectPath, relativePath);
expect({
path: fullPath,
exists: fs.existsSync(fullPath),
}).toStrictEqual({ path: fullPath, exists: false });
});
};

/**
* Expect that all required files exist for PHP.
*
* @param requiredFiles List of files that should be accessible by PHP.
* @param documentRoot Document root of the PHP server.
* @param php NodePHP instance.
*/
const expectRequiredRootFiles = (requiredFiles, documentRoot, php) => {
requiredFiles.map((relativePath) => {
const fullPath = path.join(documentRoot, relativePath);
expect({
path: fullPath,
exists: php.fileExists(fullPath),
}).toStrictEqual({ path: fullPath, exists: true });
});
};

/**
* Test that startWPNow in "index", "plugin" and "theme" modes doesn't change anything in the project directory.
*/
test.each(['index', 'plugin', 'theme'])(
'startWPNow starts %s mode',
async (mode) => {
const exampleProjectPath = path.join(exampleDir, mode);
const projectPath = path.join(tmpExampleDirectory, mode);

const rawOptions: Partial<WPNowOptions> = {
projectPath: projectPath,
};

await startWPNow(rawOptions);

const forbiddenPaths = ['wp-config.php'];

expectForbiddenProjectFiles(forbiddenPaths, projectPath);

expect(fs.readdirSync(projectPath)).toEqual(
Copy link
Member

Choose a reason for hiding this comment

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

I think it'd be better to have explicit assertions here, instead of reading data and assigning it to a variable.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I already changed it - I'm not assigning those to variables but comparing the content of two paths.

Copy link
Member

Choose a reason for hiding this comment

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

Sure, but it's still variable data. I think it should be static, otherwise it may break unexpectedly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@danielbachhuber so would you use an array of paths for each toEqual() call here, instead of reading example directories?

Copy link
Member

Choose a reason for hiding this comment

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

@wojtekn Correct.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fs.readdirSync(exampleProjectPath)
);
}
);

/**
* Test that startWPNow in "wp-content" mode mounts required files and directories, and
* that required files exist for PHP.
*/
test('startWPNow starts wp-content mode', async () => {
const projectPath = path.join(tmpExampleDirectory, 'wp-content');

const rawOptions: Partial<WPNowOptions> = {
projectPath: projectPath,
};

const { php, options: wpNowOptions } = await startWPNow(rawOptions);

const mountPointPaths = [
'database',
'db.php',
'mu-plugins',
'plugins/sqlite-database-integration',
];

expectEmptyMountPoints(mountPointPaths, projectPath);

const forbiddenPaths = ['wp-config.php'];

expectForbiddenProjectFiles(forbiddenPaths, projectPath);

const requiredFiles = [
'wp-content/db.php',
'wp-content/mu-plugins/0-allow-wp-org.php',
'playground-consts.json',
];

expectRequiredRootFiles(requiredFiles, wpNowOptions.documentRoot, php);
});

/**
* Test that startWPNow in "wordpress" mode mounts required files and directories, and
* that required files exist for PHP.
*/
test('startWPNow starts wordpress mode', async () => {
const projectPath = path.join(tmpExampleDirectory, 'wordpress');

const rawOptions: Partial<WPNowOptions> = {
projectPath: projectPath,
};

const { php, options: wpNowOptions } = await startWPNow(rawOptions);

const mountPointPaths = [
'wp-content/database',
'wp-content/db.php',
'wp-content/mu-plugins',
'wp-content/plugins/sqlite-database-integration',
];

expectEmptyMountPoints(mountPointPaths, projectPath);

const requiredFiles = [
'wp-content/db.php',
'wp-content/mu-plugins/0-allow-wp-org.php',
'playground-consts.json',
'wp-config.php',
];

expectRequiredRootFiles(requiredFiles, wpNowOptions.documentRoot, php);
});
});
11 changes: 6 additions & 5 deletions packages/wp-now/src/wp-now.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
isWordPressDirectory,
isWordPressDevelopDirectory,
} from './wp-playground-wordpress';
import { output } from './output';

export const enum WPNowMode {
PLUGIN = 'plugin',
Expand Down Expand Up @@ -115,7 +116,7 @@ export default async function startWPNow(
!seemsLikeAPHPFile(fullPath)
);
} catch (e) {
console.error(e);
output?.error(e);
return false;
}
},
Expand All @@ -125,10 +126,10 @@ export default async function startWPNow(
php.chdir(documentRoot);
php.writeFile(`${documentRoot}/index.php`, `<?php echo 'Hello wp-now!';`);

console.log(`directory: ${options.projectPath}`);
console.log(`mode: ${options.mode}`);
console.log(`php: ${options.phpVersion}`);
console.log(`wp: ${options.wordPressVersion}`);
output?.log(`directory: ${options.projectPath}`);
output?.log(`mode: ${options.mode}`);
output?.log(`php: ${options.phpVersion}`);
output?.log(`wp: ${options.wordPressVersion}`);
if (options.mode === WPNowMode.INDEX) {
await runIndexMode(php, options);
return { php, options };
Expand Down