pdom comparison to test against any old sha, #138

js/grunt/Gruntfile.js

@@ -174,7 +174,7 @@ module.exports = function( grunt ) {
   grunt.registerTask( 'pdom-comparison',
     'Compare two sim versions\' pdom',
     wrapTask( async () => {
-      await PDOMComparison( grunt.option( 'repo' ) );
+      await PDOMComparison( grunt.option( 'repo' ), grunt.option( 'sha' ) );
     } ) );
 
   grunt.registerTask( 'update-gh-pages',

js/grunt/PDOMComparison.js

@@ -2,97 +2,186 @@
 
 /**
  * Compare the PDOM html of two versions of the same sim. This is a helpful test to see if there has been any changes
- * to the descriptions.
+ * to the descriptions. This will test the current working copy (even unstashed changes) against a provided sha.
+ *
+ * This task works by using the date of the provided sha to checkout the correct shas for each dependency.
+ *
+ * If the output isn't pretty for you, it may be because of gitbash on Windows. zepumph and twant found a solution by
+ * adding `export FORCE_COLOR=true` to his bashrc, see https://stackoverflow.com/questions/32742865/no-console-colors-if-using-npm-script-inside-a-git-bash-mintty/54134326#54134326
+ *
+ * Currently this script depends on CHIPPER/getPhetLibs. This is not ideal, and means that this script should really only
+ * be called from master.
  *
  * @author Michael Kauzmann (PhET Interactive Simulations)
  * @author Jesse Greenberg (PhET Interactive Simulations)
+ * @author Taylor Want (PhET Interactive Simulations)
  */
 
 'use strict';
 
 // modules
+const _ = require( 'lodash' ); // eslint-disable-line
+const assert = require( 'assert' );
 const buildLocal = require( '../common/buildLocal' );
 const execute = require( '../common/execute' );
-const puppeteer = require( 'puppeteer' );
+const getActiveRepos = require( '../common/getActiveRepos' );
+const getPhetLibs = require( '../../../chipper/js/grunt/getPhetLibs' );
 const htmlDiffer = require( 'html-differ' ); // eslint-disable-line require-statement-match
 const logger = require( 'html-differ/lib/logger' );
+const puppeteer = require( 'puppeteer' );
+const winston = require( 'winston' );
 
 // constants
 const HtmlDiffer = htmlDiffer.HtmlDiffer;
 
-module.exports = async ( repo ) => {
+module.exports = async ( repo, sha ) => {
+  assert( typeof sha === 'string', 'need a sha to compare against' );
+  winston.debug( `running pdom comparison in ${repo} between current working copy and ${sha}` );
 
+  // TODO: perennial shouldn't depend on chipper, https://github.com/phetsims/perennial/issues/138
+  const dependencies = getPhetLibs( repo );
 
-  // TODO: get working with dependencies instead of just stashed copy changes
-  // TODO: add in functions that can be executed to change the model in between tests.
+  // get the current working copy PDOM
+  const workingCopyPDOM = await launchSimAndGetPDOMText( repo );
 
-  const puppeteerTest = async () => {
+  // keep track of the repos that will need to be unstashed. This is necessary to make sure we don't apply a stash that
+  // this task didn't save to begin with,
+  // i.e. git stash -> there are no working copy changes -> git stash apply -> previous random stash popped
+  const reposThatNeedUnstashing = await stashAll( dependencies );
 
-    return new Promise( async ( resolve, reject ) => {
+  const shaDateString = await execute( 'git', [ 'show', '--no-patch', '--no-notes', '--pretty=\'%ci\'', sha ], `../${repo}` );
+  const shaDate = shaDateString.split( ' ' )[ 0 ].replace( /-/g, '.' ).replace( '\'', '' );
 
-      const browser = await puppeteer.launch();
-      const page = await browser.newPage();
+  const activeRepos = getActiveRepos();
 
-      page.on( 'console', async msg => {
+  // checkout-date.sh takes an "opt-out" list of repos that shouldn't be checked out.
+  const keepOnMaster = _.without( activeRepos, ...dependencies );
+  keepOnMaster.push( 'babel' ); // we don't need to worry about babel, this is only for running in requirejs mode
 
-        console.log( msg.text() );
-      } );
+  // checkout date will checkout all needed repos, we don't need to npm update because we are only running in requirejs mode
+  await execute( 'sh', [ './perennial/bin/checkout-date.sh', '-m', keepOnMaster, shaDate ], '../' );
 
-      page.on( 'load', async () => {
+  // get the PDOM from the old shas
+  const oldShaPDOM = await launchSimAndGetPDOMText( repo );
 
-        // TODO: find a better way to wait until the sim is loaded, we can't use postMessageOnLoad or sim.endedSimConstructionEmitter because of timing
-        setTimeout( async () => {
+  await restoreWorkingCopy( dependencies, reposThatNeedUnstashing );
 
-          const pdomText = await page.evaluate( () => {
-            return phet.joist.sim.display.accessibleDOMElement.outerHTML;
-          } );
-          browser.close();
+  const htmlDiffer = new HtmlDiffer( {
+    ignoreAttributes: [ 'id' ],
+    compareAttributesAsJSON: [],
+    ignoreWhitespaces: true,
+    ignoreComments: true,
+    ignoreEndTags: false,
+    ignoreDuplicateAttributes: false
+  } );
 
-          resolve( pdomText );
-        }, 10000 );
-      } );
+  const diff = htmlDiffer.diffHtml( workingCopyPDOM, oldShaPDOM );
 
-      page.on( 'error', msg => reject( msg ) );
-      page.on( 'pageerror', msg => reject( msg ) );
+  // TODO: better interpretation of the diff that is output. Perhaps by looking at "diff" more manually, see https://www.npmjs.com/package/html-differ
+  console.log( logger.getDiffText( diff, { charsAroundDiff: 40 } ) );
+};
 
-      try {
-        await page.goto( `${buildLocal.localTestingURL}${repo}/${repo}_en.html?brand=phet&postMessageOnLoad` );
-      }
-      catch( e ) {
-        browser.close();
-        reject( e );
-      }
-    } );
-  };
 
-  // test current working copy
-  const workingCopy = await puppeteerTest();
+/**
+ * Checkout master again and unstash changes if they apply
+ * @param {Array.<string>} repos
+ * @param {Array.<string>} reposThatNeedUnstashing
+ * @returns {Promise<void>}
+ */
+const restoreWorkingCopy = async ( repos, reposThatNeedUnstashing ) => {
 
-  // TODO: this only stashes the single repo
+  for ( let i = 0; i < repos.length; i++ ) {
+    const repo = repos[ i ];
+    await execute( 'git', [ 'checkout', 'master' ], `../${repo}` );
+    if ( reposThatNeedUnstashing.includes( repo ) ) {
 
-  await execute( 'git', [ 'diff-index', '--quiet', 'HEAD', '--' ], `../${repo}` );
+      // stash working copy changes
+      await execute( 'git', [ 'stash', 'apply' ], `../${repo}` );
+    }
+  }
+};
 
-  // If there are no local copy changes, then we should not apply stash changes after master's test, so keep track.
-  // An exit code of 0 means that there are no working copy changes in the repo
-  const shouldStash = await execute( 'echo', [ '"$?"' ] ) !== 0;
+/**
+ * Stash working copy changes, keeping track of any repo stashed so it can be unstashed
+ * @param {Array.<string>} repos
+ * @returns {Promise<Array.<string>>} - the repos that will need unstashing
+ */
+const stashAll = async repos => {
 
-  // stash working copy changes
-  shouldStash && await execute( 'git', [ 'stash' ], `../${repo}` );
+  // {Array.<string>} keep track of which repos were stashed
+  const needsUnstashing = [];
 
-  const master = await puppeteerTest();
-  shouldStash && await execute( 'git', [ 'stash', 'apply' ], `../${repo}` );
+  for ( let i = 0; i < repos.length; i++ ) {
+    const repo = repos[ i ];
 
-  const htmlDiffer = new HtmlDiffer( {
-    ignoreAttributes: [ 'id' ],
-    compareAttributesAsJSON: [],
-    ignoreWhitespaces: true,
-    ignoreComments: true,
-    ignoreEndTags: false,
-    ignoreDuplicateAttributes: false
-  } );
+    // If there are no local copy changes, then we should not apply stash changes after the old sha's test, so keep track.
+    // An exit code of 0 means that there are no working copy changes in the repo
 
-  const diff = htmlDiffer.diffHtml( workingCopy, master );
+    let shouldStash;
+    try {
 
-  // TODO: better interpretation of the diff that is output. Perhaps by looking at "diff" more manually, see https://www.npmjs.com/package/html-differ
-  logger.logDiffText( diff, { charsAroundDiff: 40 } );
+      await execute( 'git', [ 'diff-index', '--quiet', 'HEAD', '--' ], `../${repo}` );
+      shouldStash = false;
+    }
+    catch( e ) {
+      winston.debug( `stashing needed in ${repo}` );
+      shouldStash = true;
+    }
+    if ( shouldStash ) {
+
+      // TODO: why are all repos being pushed here!?!?
+      needsUnstashing.push( repo );
+
+      // stash working copy changes
+      await execute( 'git', [ 'stash' ], `../${repo}` );
+    }
+  }
+  return needsUnstashing;
+};
+
+
+/**
+ * Launch a chrome version, run the simulation, and get the PDOM from the simulation.
+ * TODO: add in functions that can be executed to change the model in between tests.
+ * TODO: maybe we could fuzz a few frames, and then test while setting the random seed to be the same for all pages
+ * @param repo
+ * @returns {Promise<string>}
+ */
+const launchSimAndGetPDOMText = async repo => {
+
+  return new Promise( async ( resolve, reject ) => {
+
+    const browser = await puppeteer.launch();
+    const page = await browser.newPage();
+
+    page.on( 'console', async msg => {
+
+      console.log( msg.text() );
+    } );
+
+    page.on( 'load', async () => {
+
+      // TODO: find a better way to wait until the sim is loaded, we can't use postMessageOnLoad or sim.endedSimConstructionEmitter because of timing
+      setTimeout( async () => {
+
+        const pdomText = await page.evaluate( () => {
+          return phet.joist.sim.display.accessibleDOMElement.outerHTML;
+        } );
+        browser.close();
+
+        resolve( pdomText );
+      }, 10000 );
+    } );
+
+    page.on( 'error', msg => reject( msg ) );
+    page.on( 'pageerror', msg => reject( msg ) );
+
+    try {
+      await page.goto( `${buildLocal.localTestingURL}${repo}/${repo}_en.html?brand=phet&postMessageOnLoad` );
+    }
+    catch( e ) {
+      browser.close();
+      reject( e );
+    }
+  } );
 };