Skip to content

Commit

Permalink
🏗 Rename --compiled to --minified in AMP testing tasks (ampprojec…
Browse files Browse the repository at this point in the history
  • Loading branch information
rsimha authored Aug 17, 2021
1 parent cf9c845 commit aaa3696
Show file tree
Hide file tree
Showing 37 changed files with 86 additions and 89 deletions.
10 changes: 4 additions & 6 deletions build-system/common/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,14 @@ const {log, logLocalDev} = require('./logging');

/**
* Performs a clean build of the AMP runtime in testing mode.
* Used by `amp e2e|integration|visual_diff`.
* Used by `amp e2e|integration|visual-diff`.
*
* @param {boolean} opt_compiled pass true to build the compiled runtime
* (`amp dist` instead of `amp build`). Otherwise uses the value of
* --compiled to determine which build to generate.
* @param {boolean} opt_minified builds the minified runtime
* @return {Promise<void>}
*/
async function buildRuntime(opt_compiled = false) {
async function buildRuntime(opt_minified = false) {
await clean();
if (argv.compiled || opt_compiled === true) {
if (argv.minified || opt_minified === true) {
execOrDie(`amp dist --fortesting`);
} else {
execOrDie(`amp build --fortesting`);
Expand Down
2 changes: 1 addition & 1 deletion build-system/compile/build-constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const testTasks = [
];
const isTestTask = testTasks.some((task) => argv._.includes(task));
const isProd = argv._.includes('dist') && !argv.fortesting;
const isMinified = argv._.includes('dist') || !!argv.compiled;
const isMinified = argv._.includes('dist') || !!argv.minified;

/**
* Build time constants. Used by babel but hopefully one day directly by the bundlers..
Expand Down
8 changes: 4 additions & 4 deletions build-system/pr-check/browser-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ function pushBuildWorkflow() {
try {
timedExecOrThrow(`amp unit --report --${browser}`, 'Unit tests failed!');
timedExecOrThrow(
`amp integration --report --nobuild --compiled --${browser}`,
`amp integration --report --nobuild --minified --${browser}`,
'Integration tests failed!'
);
timedExecOrThrow(
`amp e2e --report --nobuild --compiled --browsers=${browser}`,
`amp e2e --report --nobuild --minified --browsers=${browser}`,
'End-to-end tests failed!'
);
} catch (e) {
Expand Down Expand Up @@ -75,13 +75,13 @@ function prBuildWorkflow() {
timedExecOrDie(`amp unit --${browser}`);
}
if (buildTargetsInclude(Targets.RUNTIME, Targets.INTEGRATION_TEST)) {
timedExecOrDie(`amp integration --nobuild --compiled --${browser}`);
timedExecOrDie(`amp integration --nobuild --minified --${browser}`);
}
if (
buildTargetsInclude(Targets.RUNTIME, Targets.E2E_TEST) &&
['safari', 'firefox'].includes(browser) // E2E tests can't be run on Edge.
) {
timedExecOrDie(`amp e2e --nobuild --compiled --browsers=${browser}`);
timedExecOrDie(`amp e2e --nobuild --minified --browsers=${browser}`);
}
}

Expand Down
4 changes: 2 additions & 2 deletions build-system/pr-check/e2e-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ function pushBuildWorkflow() {
try {
generateCircleCiShardTestFileList(e2eTestPaths);
timedExecOrThrow(
`amp e2e --nobuild --headless --compiled --report --filelist ${TEST_FILES_LIST_FILE_NAME}`,
`amp e2e --nobuild --headless --minified --report --filelist ${TEST_FILES_LIST_FILE_NAME}`,
'End-to-end tests failed!'
);
} catch (e) {
Expand All @@ -58,7 +58,7 @@ function prBuildWorkflow() {
if (buildTargetsInclude(Targets.RUNTIME, Targets.E2E_TEST)) {
generateCircleCiShardTestFileList(e2eTestPaths);
timedExecOrDie(
`amp e2e --nobuild --headless --compiled --filelist ${TEST_FILES_LIST_FILE_NAME}`
`amp e2e --nobuild --headless --minified --filelist ${TEST_FILES_LIST_FILE_NAME}`
);
} else {
skipDependentJobs(
Expand Down
2 changes: 1 addition & 1 deletion build-system/pr-check/experiment-e2e-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ function runExperimentTests(config) {
const reportFlag = isPushBuild() ? '--report' : '';
generateCircleCiShardTestFileList(e2eTestPaths);
timedExecOrThrow(
`amp e2e --nobuild --compiled --headless ${experimentFlag} ${defineFlag} ${reportFlag} --filelist ${TEST_FILES_LIST_FILE_NAME}`
`amp e2e --nobuild --minified --headless ${experimentFlag} ${defineFlag} ${reportFlag} --filelist ${TEST_FILES_LIST_FILE_NAME}`
);
} catch (e) {
if (e.status) {
Expand Down
2 changes: 1 addition & 1 deletion build-system/pr-check/experiment-integration-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ function runExperimentTests(config) {
const experimentFlag = `--experiment ${experiment}`;
const reportFlag = isPushBuild() ? '--report' : '';
timedExecOrThrow(
`amp integration --nobuild --compiled --headless ${experimentFlag} ${defineFlag} ${reportFlag}`
`amp integration --nobuild --minified --headless ${experimentFlag} ${defineFlag} ${reportFlag}`
);
} catch (e) {
if (e.status) {
Expand Down
2 changes: 1 addition & 1 deletion build-system/pr-check/ie-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const jobName = 'ie-tests.js';
*/
function pushBuildWorkflow() {
timedExecOrDie('amp dist --fortesting');
timedExecOrDie('amp integration --nobuild --compiled --ie');
timedExecOrDie('amp integration --nobuild --minified --ie');
}

/**
Expand Down
4 changes: 2 additions & 2 deletions build-system/pr-check/module-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ function prependConfig() {
*/
function pushBuildWorkflow() {
prependConfig();
timedExecOrDie('amp integration --nobuild --compiled --headless --esm');
timedExecOrDie('amp integration --nobuild --minified --headless --esm');
}

/**
Expand All @@ -55,7 +55,7 @@ function prBuildWorkflow() {
if (buildTargetsInclude(Targets.RUNTIME, Targets.INTEGRATION_TEST)) {
prependConfig();
timedExecOrDie(
`amp integration --nobuild --compiled --headless --esm --config=${argv.config}`
`amp integration --nobuild --minified --headless --esm --config=${argv.config}`
);
} else {
skipDependentJobs(
Expand Down
4 changes: 2 additions & 2 deletions build-system/pr-check/nomodule-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ function pushBuildWorkflow() {
prependConfig();
try {
timedExecOrThrow(
`amp integration --nobuild --headless --compiled --report --config=${argv.config}`,
`amp integration --nobuild --headless --minified --report --config=${argv.config}`,
'Integration tests failed!'
);
} catch (e) {
Expand All @@ -69,7 +69,7 @@ function prBuildWorkflow() {
if (buildTargetsInclude(Targets.RUNTIME, Targets.INTEGRATION_TEST)) {
prependConfig();
timedExecOrDie(
`amp integration --nobuild --compiled --headless --config=${argv.config}`
`amp integration --nobuild --minified --headless --config=${argv.config}`
);
} else {
skipDependentJobs(
Expand Down
12 changes: 6 additions & 6 deletions build-system/server/amp4test.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ app.use('/compose-doc', function (req, res) {
const {body, css, experiments, extensions, spec} = req.query;

const frameHtml =
SERVE_MODE == 'compiled'
SERVE_MODE == 'minified'
? 'dist.3p/current-min/frame.html'
: 'dist.3p/current/frame.max.html';

Expand Down Expand Up @@ -239,7 +239,7 @@ function composeDocument(config) {

const m = mode || SERVE_MODE;
const cdn = m === 'cdn';
const compiled = m === 'compiled';
const minified = m === 'minified';

const cssTag = css ? `<style amp-custom>${css}</style>` : '';

Expand All @@ -254,23 +254,23 @@ function composeDocument(config) {
'<style amp-boilerplate>body{-webkit-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-moz-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-ms-animation:-amp-start 8s steps(1,end) 0s 1 normal both;animation:-amp-start 8s steps(1,end) 0s 1 normal both}@-webkit-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-moz-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-ms-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-o-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}</style><noscript><style amp-boilerplate>body{-webkit-animation:none;-moz-animation:none;-ms-animation:none;animation:none}</style></noscript>';
runtime = cdn
? 'https://cdn.ampproject.org/v0.js'
: `/dist/${compiled ? 'v0' : 'amp'}.js`;
: `/dist/${minified ? 'v0' : 'amp'}.js`;
break;
case 'amp4ads':
canonical = '';
boilerplate =
'<style amp4ads-boilerplate>body{visibility:hidden}</style>';
runtime = cdn
? 'https://cdn.ampproject.org/amp4ads-v0.js'
: `/dist/${compiled ? 'amp4ads-v0' : 'amp-inabox'}.js`;
: `/dist/${minified ? 'amp4ads-v0' : 'amp-inabox'}.js`;
break;
case 'amp4email':
canonical = '';
boilerplate =
'<style amp4email-boilerplate>body{visibility:hidden}</style>';
runtime = cdn
? 'https://cdn.ampproject.org/v0.js'
: `/dist/${compiled ? 'v0' : 'amp'}.js`;
: `/dist/${minified ? 'v0' : 'amp'}.js`;
break;
default:
throw new Error('Unrecognized AMP spec: ' + spec);
Expand All @@ -287,7 +287,7 @@ function composeDocument(config) {
const version = tuple[1] || '0.1';
const src = cdn
? `https://cdn.ampproject.org/v0/${name}-${version}.js`
: `/dist/v0/${name}-${version}.${compiled ? '' : 'max.'}js`;
: `/dist/v0/${name}-${version}.${minified ? '' : 'max.'}js`;
const type = CUSTOM_TEMPLATES.includes(name)
? 'custom-template'
: 'custom-element';
Expand Down
2 changes: 1 addition & 1 deletion build-system/server/app-index/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const jsModes = [
changes.`,
},
{
value: 'compiled',
value: 'minified',
description: html`
Minified AMP JavaScript is served from the local server. This is only
available after running <code>amp dist --fortesting</code>
Expand Down
8 changes: 4 additions & 4 deletions build-system/server/app-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ function setServeMode(modeOptions) {
modeOptions = minimist(process.argv.slice(2), {string: ['rtv']});
}

if (modeOptions.compiled) {
serveMode = 'compiled';
if (modeOptions.minified) {
serveMode = 'minified';
} else if (modeOptions.esm) {
serveMode = 'esm';
} else if (modeOptions.cdn) {
Expand All @@ -61,7 +61,7 @@ function setServeMode(modeOptions) {
*/
function logServeMode() {
const serveMode = getServeMode();
if (serveMode == 'compiled') {
if (serveMode == 'minified') {
log(green('Serving'), cyan('minified'), green('JS'));
} else if (serveMode == 'esm') {
log(green('Serving'), cyan('ESM'), green('JS'));
Expand Down Expand Up @@ -151,7 +151,7 @@ function replaceUrls(mode, html, hostName = '') {
// If you need to add URL mapping logic, please don't do it in this function.
// Instead, do so in the `cdn.ts` module required by `replaceCdnJsUrls()`

const useMaxNames = mode !== 'compiled';
const useMaxNames = mode !== 'minified';
if (!useMaxNames) {
// TODO(alanorozco): This should be handled in new-server as well.
html = html.replace(
Expand Down
12 changes: 6 additions & 6 deletions build-system/server/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ app.use((req, res, next) => {
*/
function isValidServeMode(serveMode) {
return (
['default', 'compiled', 'cdn', 'esm'].includes(serveMode) ||
['default', 'minified', 'cdn', 'esm'].includes(serveMode) ||
isRtvMode(serveMode)
);
}
Expand All @@ -149,7 +149,7 @@ app.get('/serve_mode=:mode', (req, res) => {
});

if (argv._.includes('integration') && !argv.nobuild) {
setServeMode('compiled');
setServeMode('minified');
}

if (!(argv._.includes('unit') || argv._.includes('integration'))) {
Expand All @@ -158,7 +158,7 @@ if (!(argv._.includes('unit') || argv._.includes('integration'))) {
}

// Changes the current serve mode via query param
// e.g. /serve_mode_change?mode=(default|compiled|cdn|<RTV_NUMBER>)
// e.g. /serve_mode_change?mode=(default|minified|cdn|<RTV_NUMBER>)
// (See ./app-index/settings.js)
app.get('/serve_mode_change', (req, res) => {
const {mode} = req.query;
Expand Down Expand Up @@ -579,7 +579,7 @@ const liveListDocs = Object.create(null);
app.use('/examples/live-list-update(-reverse)?.amp.html', (req, res, next) => {
const mode = SERVE_MODE;
let liveListDoc = liveListDocs[req.baseUrl];
if (mode != 'compiled' && mode != 'default') {
if (mode != 'minified' && mode != 'default') {
// Only handle compile(prev min)/default (prev max) mode
next();
return;
Expand Down Expand Up @@ -1443,7 +1443,7 @@ window.addEventListener('beforeunload', (evt) => {
}

app.get('/dist/ww.(m?js)', async (req, res, next) => {
// Special case for entry point script url. Use compiled for testing
// Special case for entry point script url. Use minified for testing
const mode = SERVE_MODE;
const fileName = path.basename(req.path);
if (await passthroughServeModeCdn(res, fileName)) {
Expand Down Expand Up @@ -1560,7 +1560,7 @@ function generateInfo(filePath) {
'<h3></h3>' +
'<h3><a href = /serve_mode=default>' +
'Change to DEFAULT mode (unminified JS)</a></h3>' +
'<h3><a href = /serve_mode=compiled>' +
'<h3><a href = /serve_mode=minified>' +
'Change to COMPILED mode (minified JS)</a></h3>' +
'<h3><a href = /serve_mode=cdn>' +
'Change to CDN mode (prod JS)</a></h3>'
Expand Down
8 changes: 4 additions & 4 deletions build-system/server/lazy-build.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ const vendorBundles = generateBundles();
* @return {string}
*/
function maybeGetUnminifiedName(bundles, name) {
if (argv.compiled) {
if (argv.minified) {
for (const key of Object.keys(bundles)) {
if (
key == name ||
Expand Down Expand Up @@ -98,7 +98,7 @@ async function build(bundles, name, buildFunc) {
bundle.watched = true;
bundle.pendingBuild = buildFunc(bundles, name, {
watch: true,
minify: argv.compiled,
minify: argv.minified,
onWatchBuild: async (bundlePromise) => {
bundle.pendingBuild = bundlePromise;
await bundlePromise;
Expand All @@ -118,7 +118,7 @@ async function build(bundles, name, buildFunc) {
* @return {Promise<void>}
*/
async function lazyBuildExtensions(req, _res, next) {
const matcher = argv.compiled
const matcher = argv.minified
? /\/dist\/v0\/([^\/]*)\.js/ // '/dist/v0/*.js'
: /\/dist\/v0\/([^\/]*)\.max\.js/; // '/dist/v0/*.max.js'
await lazyBuild(req.url, matcher, extensionBundles, doBuildExtension, next);
Expand Down Expand Up @@ -146,7 +146,7 @@ async function lazyBuildJs(req, _res, next) {
* @return {Promise<void>}
*/
async function lazyBuild3pVendor(req, _res, next) {
const matcher = argv.compiled
const matcher = argv.minified
? new RegExp(`\\/dist\\.3p\\/${VERSION}\\/vendor\\/([^\/]*)\\.js`) // '/dist.3p/21900000/vendor/*.js'
: /\/dist\.3p\/current\/vendor\/([^\/]*)\.max\.js/; // '/dist.3p/current/vendor/*.max.js'
await lazyBuild(req.url, matcher, vendorBundles, doBuild3pVendor, next);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ function sxgTransform(node: posthtml.Node, options: OptionSet = {}): posthtml.No
return node;
}

if (options.compiled) {
if (options.minified) {
const src = node.attrs.src;
node.attrs.src = src.replace('.js', '.sxg.js');
} else {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
{
"compiled": true
"minified": true
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
{
"compiled": false
"minified": false
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
{
"compiled": false
"minified": false
}
2 changes: 1 addition & 1 deletion build-system/server/new-server/transforms/transform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const defaultTransformConfig = {
esm: ESM,
port: PORT,
fortesting: FOR_TESTING,
useMaxNames: !argv.compiled,
useMaxNames: !argv.minified,
};

const transforms = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
* options.json, then add the field to this interface.
*/
export interface OptionSet {
compiled?: boolean;
minified?: boolean;
esm?: boolean;
port?: number;
fortesting?: boolean;
Expand Down
12 changes: 6 additions & 6 deletions build-system/server/test/app-utils.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
const test = require('ava');
const {replaceUrls, toInaboxDocument} = require('../app-utils');

test('replaceUrls("compiled", ...)', async (t) => {
const mode = 'compiled';
test('replaceUrls("minified", ...)', async (t) => {
const mode = 'minified';
t.is(
replaceUrls(
mode,
Expand Down Expand Up @@ -75,8 +75,8 @@ test('replaceUrls("compiled", ...)', async (t) => {
);
});

test('replaceUrls("compiled", ..., hostName)', async (t) => {
const mode = 'compiled';
test('replaceUrls("minified", ..., hostName)', async (t) => {
const mode = 'minified';
const hostName = 'https://foo.bar';
t.is(
replaceUrls(
Expand Down Expand Up @@ -340,8 +340,8 @@ test('toInaboxDocument(...)', async (t) => {
);
});

test('replaceUrls("compiled", toInaboxDocument(...))', async (t) => {
const mode = 'compiled';
test('replaceUrls("minified", toInaboxDocument(...))', async (t) => {
const mode = 'minified';
const hostName = '';
t.is(
replaceUrls(
Expand Down
Loading

0 comments on commit aaa3696

Please sign in to comment.