Turn on esm tests #28336

erwinmombay · 2020-05-12T14:52:42Z

No description provided.

rsimha

Thanks for picking this up, @erwinmombay! I've made some preliminary comments in case they might help you debug / troubleshoot.

rsimha · 2020-05-12T23:06:04Z

build-system/pr-check/esm-tests.js

+
+  if (!isTravisPullRequestBuild()) {
+    timedExecOrDie('gulp update-packages');
+    timedExecOrDie('gulp dist --fortesting --esm');


No need to compile here. You can download what was built in the ESM Dist job from the build stage (added in #27705). This will shave off ~10 mins from the total build time.

amphtml/build-system/pr-check/utils.js

Lines 338 to 344 in 6913469

/**

* Downloads and unzips esm dist output from storage

* @param {string} functionName

*/

function downloadEsmDistOutput(functionName) {

downloadOutput_(functionName, ESM_DIST_OUTPUT_FILE, BUILD_OUTPUT_DIRS);

}

Recommended order of steps:

downloadEsmDistOutput(FILENAME); timedExecOrDie('gulp update-packages'); timedExecOrDie('gulp integration --nobuild --compiled --headless');

build-system/pr-check/esm-tests.js

rsimha · 2020-05-12T23:11:08Z

build-system/pr-check/esm-tests.js

+      timedExecOrDie('gulp update-packages');
+      timedExecOrDie('gulp dist --fortesting --esm');
+      timedExecOrDie(
+        'gulp integration --nobuild --compiled --headless --coverage'


Drop --coverage. (Coverage mode only works for unminified code.)

rsimha · 2020-05-12T23:15:27Z

build-system/pr-check/esm-tests.js

+  if (!isTravisPullRequestBuild()) {
+    timedExecOrDie('gulp update-packages');
+    timedExecOrDie('gulp dist --fortesting --esm');
+    timedExecOrDie('gulp integration --nobuild --compiled --headless');


Couple comments:

This will not start up the test server that's built-in to gulp integration in --esm mode. Is this a problem? You may need to make a change to the integration test runner to launch the server in esm mode. See 🏗✨ Add esm support for gulp serve via a new typescript-based server module #27340 (comment) for details.

This will break test status reporting by merging esm and non-esm results. You'd need to add a new mode here:

amphtml/build-system/tasks/report-test-status.js

Lines 27 to 44 in 6913469

const IS_GULP_INTEGRATION = argv._[0] === 'integration';

const IS_GULP_UNIT = argv._[0] === 'unit';

const IS_GULP_E2E = argv._[0] === 'e2e';

const IS_LOCAL_CHANGES = !!argv.local_changes;

const IS_SAUCELABS = !!argv.saucelabs;

const IS_SAUCELABS_STABLE = !!argv.saucelabs && !!argv.stable;

const IS_SAUCELABS_BETA = !!argv.saucelabs && !!argv.beta;

const IS_SINGLE_PASS = !!argv.single_pass;

const TEST_TYPE_SUBTYPES = new Map([

[

'integration',

['local', 'single-pass', 'saucelabs-beta', 'saucelabs-stable'],

],

['unit', ['local', 'local-changes', 'saucelabs']],

['e2e', ['local']],

]);

lgtm-com · 2020-09-22T17:14:00Z

This pull request introduces 2 alerts when merging 0b3203c into f3612a9 - view on LGTM.com

new alerts:

1 for Useless assignment to local variable
1 for Useless conditional

amp-owners-bot · 2020-10-20T16:25:47Z

Hey @rsimha! These files were changed:

build-system/pr-check/esm-tests.js
build-system/pr-check/utils.js

build-system/pr-check/esm-tests.js

CLAassistant · 2020-10-29T20:51:03Z

All committers have signed the CLA.

build-system/pr-check/utils.js

…mpproject#27576)" This reverts commit f801a93.

… for regex replace

rsimha

🎉

* temp * temp * add conditional to the css removal * add travis entry * add esm-tests.js file * minor fix to add coverage * remove compiled to test * add compiled back in * Revert "🏗♻️ Consolidate all `babel` configs into `babel.config.js` (ampproject#27576)" This reverts commit f801a93. * temp * temp * reset this to be close to master * temp * temp * temp * temp * for non dev builds (compiled, rtv) take into account the module build for regex replace * temp * remove debuggers * temp * apply recs * remove runner.js * apply recs * add --esm flag to integration run * download dist output as we need f.js/integration.js from dist nomodule * allow cors explicitly for mjs files GET requests * add flag to overwrite when unzipping * fix test type inference * apply recs

googlebot added the cla: yes label May 12, 2020

rsimha reviewed May 12, 2020

View reviewed changes

erwinmombay force-pushed the turn-on-esm-tests branch from 9e87060 to b837407 Compare June 16, 2020 14:47

erwinmombay force-pushed the turn-on-esm-tests branch 3 times, most recently from dcf1124 to 0b3203c Compare September 22, 2020 16:52

xiexr151e mentioned this pull request Sep 22, 2020

✅ Add integration tests for SXG build. #30356

Closed

erwinmombay mentioned this pull request Sep 23, 2020

fix gulp integration and server so that it can work with mjs files #30292

Merged

erwinmombay mentioned this pull request Oct 20, 2020

pass fixture files through the postHTML transformer code #30681

Merged

erwinmombay force-pushed the turn-on-esm-tests branch 5 times, most recently from b903281 to 9b399e8 Compare October 20, 2020 16:25

erwinmombay marked this pull request as ready for review October 20, 2020 16:25

amp-owners-bot bot requested a review from jridgewell October 20, 2020 16:25

erwinmombay force-pushed the turn-on-esm-tests branch 2 times, most recently from eda171c to 30d778c Compare October 20, 2020 16:30

erwinmombay requested review from estherkim and removed request for jridgewell October 20, 2020 17:55

erwinmombay force-pushed the turn-on-esm-tests branch 2 times, most recently from 338b1fd to 5f8a783 Compare October 21, 2020 17:27

erwinmombay commented Oct 21, 2020

View reviewed changes

build-system/pr-check/esm-tests.js Show resolved Hide resolved

erwinmombay force-pushed the turn-on-esm-tests branch from 5f8a783 to d1b79c4 Compare October 29, 2020 20:50

erwinmombay force-pushed the turn-on-esm-tests branch 2 times, most recently from 7303998 to bc92991 Compare November 2, 2020 20:05

erwinmombay commented Nov 2, 2020

View reviewed changes

build-system/pr-check/utils.js Show resolved Hide resolved

erwinmombay added 22 commits November 2, 2020 13:11

remove compiled to test

b3395db

add compiled back in

8c4cea5

Revert "🏗♻️ Consolidate all babel configs into babel.config.js (a…

7c2ce20

…mpproject#27576)" This reverts commit f801a93.

temp

a2b98d5

temp

4e45bcc

reset this to be close to master

b504d75

temp

81b5cad

temp

e0107a5

temp

bce5f54

temp

309ee64

for non dev builds (compiled, rtv) take into account the module build…

5b4a8ae

… for regex replace

temp

db9d398

remove debuggers

ea84a65

temp

042e339

apply recs

fada8a7

remove runner.js

b7f5f86

apply recs

de569a0

add --esm flag to integration run

7cd6706

download dist output as we need f.js/integration.js from dist nomodule

a27b2bb

allow cors explicitly for mjs files GET requests

f07d127

add flag to overwrite when unzipping

d50d867

fix test type inference

8c43126

erwinmombay force-pushed the turn-on-esm-tests branch from 3638dba to 721dd9e Compare November 2, 2020 21:11

apply recs

e9bf0c3

erwinmombay force-pushed the turn-on-esm-tests branch from 721dd9e to e9bf0c3 Compare November 2, 2020 21:12

rsimha approved these changes Nov 2, 2020

View reviewed changes

erwinmombay merged commit f479ea6 into ampproject:master Nov 2, 2020

erwinmombay mentioned this pull request Nov 9, 2020

wg-performance Status Update 2020-11-09 ampproject/wg-performance#59

Closed

ampprojectbot added the PR use: In Beta / Experimental label Nov 11, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Turn on esm tests #28336

Turn on esm tests #28336

erwinmombay commented May 12, 2020

rsimha left a comment

rsimha May 12, 2020 •

edited

Loading

rsimha May 12, 2020 •

edited

Loading

rsimha May 12, 2020 •

edited

Loading

lgtm-com bot commented Sep 22, 2020

amp-owners-bot bot commented Oct 20, 2020 •

edited

Loading

CLAassistant commented Oct 29, 2020 •

edited

Loading

rsimha left a comment

	/**
	* Downloads and unzips esm dist output from storage
	* @param {string} functionName
	*/
	function downloadEsmDistOutput(functionName) {
	downloadOutput_(functionName, ESM_DIST_OUTPUT_FILE, BUILD_OUTPUT_DIRS);
	}

	const IS_GULP_INTEGRATION = argv._[0] === 'integration';
	const IS_GULP_UNIT = argv._[0] === 'unit';
	const IS_GULP_E2E = argv._[0] === 'e2e';

	const IS_LOCAL_CHANGES = !!argv.local_changes;
	const IS_SAUCELABS = !!argv.saucelabs;
	const IS_SAUCELABS_STABLE = !!argv.saucelabs && !!argv.stable;
	const IS_SAUCELABS_BETA = !!argv.saucelabs && !!argv.beta;
	const IS_SINGLE_PASS = !!argv.single_pass;

	const TEST_TYPE_SUBTYPES = new Map([
	[
	'integration',
	['local', 'single-pass', 'saucelabs-beta', 'saucelabs-stable'],
	],
	['unit', ['local', 'local-changes', 'saucelabs']],
	['e2e', ['local']],
	]);

Turn on esm tests #28336

Turn on esm tests #28336

Conversation

erwinmombay commented May 12, 2020

rsimha left a comment

Choose a reason for hiding this comment

rsimha May 12, 2020 • edited Loading

Choose a reason for hiding this comment

rsimha May 12, 2020 • edited Loading

Choose a reason for hiding this comment

rsimha May 12, 2020 • edited Loading

Choose a reason for hiding this comment

lgtm-com bot commented Sep 22, 2020

amp-owners-bot bot commented Oct 20, 2020 • edited Loading

CLAassistant commented Oct 29, 2020 • edited Loading

rsimha left a comment

Choose a reason for hiding this comment

rsimha May 12, 2020 •

edited

Loading

rsimha May 12, 2020 •

edited

Loading

rsimha May 12, 2020 •

edited

Loading

amp-owners-bot bot commented Oct 20, 2020 •

edited

Loading

CLAassistant commented Oct 29, 2020 •

edited

Loading