Factor out and dedup 'validate' step of presubmit (gpuweb#2608)

Prior to this, the gen_listings script actually did a lot of bonus work
on top of generating listings: it validated a bunch of things about test
files and tests. However this is (1) misleading and (2) called twice
during the build process, once for standalone and once for wpt.

This change splits validation out into a separate script, and also runs
both gen_listings and validate only once instead of twice.

On my machine, gen_listings takes 0.75 seconds without validation, but
validation adds about 16s. So now, even though crawling happens twice
(2 originally, minus 1 by deduplicating, plus 1 in the validate tool)
this shaves that off the presubmit time: before: 143s, after: 129s.
OK, so it's still really slow, but it's better.

Also, removes the quiet-grunt setup that made this really hard to see
due to quieting all of the build steps. It was added because development
required constant command-line builds, but now that we have the dev
server I don't think anyone is doing this anymore.

Also also, fixes a bug in gen_version which was preventing it from
running without an existing standalone build.

Author: kainino0x

Diff for: Gruntfile.js

@@ -20,6 +20,10 @@ module.exports = function (grunt) {
         cmd: 'node',
         args: ['tools/gen_listings', 'out/', 'src/webgpu', 'src/stress', 'src/manual', 'src/unittests', 'src/demo'],
       },
+      validate: {
+        cmd: 'node',
+        args: ['tools/validate', 'src/webgpu', 'src/stress', 'src/manual', 'src/unittests', 'src/demo'],
+      },
       'generate-wpt-cts-html': {
         cmd: 'node',
         args: ['tools/gen_wpt_cts_html', 'out-wpt/cts.https.html', 'src/common/templates/cts.https.html'],
@@ -155,23 +159,16 @@ module.exports = function (grunt) {
     helpMessageTasks.push({ name, desc });
   }
 
-  grunt.registerTask('set-quiet-mode', () => {
-    grunt.log.write('Running tasks');
-    require('quiet-grunt');
-  });
-
-  grunt.registerTask('build-standalone', 'Build out/ (no checks, no WPT)', [
+  grunt.registerTask('build-standalone', 'Build out/ (no listings, no checks, no WPT)', [
     'run:build-out',
     'run:copy-assets',
     'run:generate-version',
-    'run:generate-listings',
   ]);
-  grunt.registerTask('build-wpt', 'Build out/ (no checks)', [
+  grunt.registerTask('build-wpt', 'Build out-wpt/ (no checks; run after generate-listings)', [
     'run:build-out-wpt',
     'run:copy-assets-wpt',
     'run:autoformat-out-wpt',
     'run:generate-version',
-    'run:generate-listings',
     'copy:out-wpt-generated',
     'copy:out-wpt-htmlfiles',
     'run:generate-wpt-cts-html',
@@ -181,8 +178,9 @@ module.exports = function (grunt) {
   });
 
   registerTaskAndAddToHelp('pre', 'Run all presubmit checks: standalone+wpt+typecheck+unittest+lint', [
-    'set-quiet-mode',
     'clean',
+    'run:generate-listings',
+    'run:validate',
     'build-standalone',
     'build-wpt',
     'run:build-out-node',
@@ -194,23 +192,24 @@ module.exports = function (grunt) {
     'run:tsdoc-treatWarningsAsErrors',
   ]);
   registerTaskAndAddToHelp('standalone', 'Build standalone and typecheck', [
-    'set-quiet-mode',
+    'run:generate-listings',
     'build-standalone',
     'build-done-message',
+    'run:validate',
     'ts:check',
   ]);
   registerTaskAndAddToHelp('wpt', 'Build for WPT and typecheck', [
-    'set-quiet-mode',
+    'run:generate-listings',
     'build-wpt',
     'build-done-message',
+    'run:validate',
     'ts:check',
   ]);
   registerTaskAndAddToHelp('unittest', 'Build standalone, typecheck, and unittest', [
     'standalone',
     'run:unittest',
   ]);
   registerTaskAndAddToHelp('check', 'Just typecheck', [
-    'set-quiet-mode',
     'ts:check',
   ]);
 

Diff for: package-lock.json

@@ -68,7 +68,6 @@
     "pngjs": "^6.0.0",
     "portfinder": "^1.0.32",
     "prettier": "~2.1.2",
-    "quiet-grunt": "^0.2.3",
     "screenshot-ftw": "^1.0.5",
     "serve-index": "^1.9.1",
     "ts-node": "^9.0.0",

Diff for: package.json

@@ -43,10 +43,7 @@ async function crawlFilesRecursively(dir: string): Promise<string[]> {
   );
 }
 
-export async function crawl(
-  suiteDir: string,
-  validate: boolean = true
-): Promise<TestSuiteListingEntry[]> {
+export async function crawl(suiteDir: string, validate: boolean): Promise<TestSuiteListingEntry[]> {
   if (!fs.existsSync(suiteDir)) {
     console.error(`Could not find ${suiteDir}`);
     process.exit(1);

Diff for: src/common/tools/crawl.ts

@@ -13,7 +13,6 @@ into OUT_DIR/{suite}/listing.js. Example:
 
 Options:
   --help          Print this message and exit.
-  --no-validate   Whether to validate test modules while crawling.
 `);
   process.exit(rc);
 }
@@ -23,11 +22,10 @@ if (argv.indexOf('--help') !== -1) {
   usage(0);
 }
 
-let validate = true;
 {
+  // Ignore old argument that is now the default
   const i = argv.indexOf('--no-validate');
   if (i !== -1) {
-    validate = false;
     argv.splice(i, 1);
   }
 }
@@ -40,10 +38,9 @@ const myself = 'src/common/tools/gen_listings.ts';
 
 const outDir = argv[2];
 
-void (async () => {
-  for (const suiteDir of argv.slice(3)) {
-    const listing = await crawl(suiteDir, validate);
-
+for (const suiteDir of argv.slice(3)) {
+  // Run concurrently for each suite (might be a tiny bit more efficient)
+  void crawl(suiteDir, false).then(listing => {
     const suite = path.basename(suiteDir);
     const outFile = path.normalize(path.join(outDir, `${suite}/listing.js`));
     fs.mkdirSync(path.join(outDir, suite), { recursive: true });
@@ -55,10 +52,12 @@ void (async () => {
 export const listing = ${JSON.stringify(listing, undefined, 2)};
 `
     );
+
+    // If there was a sourcemap for the file we just replaced, delete it.
     try {
       fs.unlinkSync(outFile + '.map');
     } catch (ex) {
       // ignore if file didn't exist
     }
-  }
-})();
+  });
+}

Diff for: src/common/tools/gen_listings.ts

@@ -0,0 +1,35 @@
+import * as process from 'process';
+
+import { crawl } from './crawl.js';
+
+function usage(rc: number): void {
+  console.error(`Usage: tools/validate [options] [SUITE_DIRS...]
+
+For each suite in SUITE_DIRS, validate some properties about the file:
+- It has a .description and .g
+- That each test:
+  - Has a test function (or is marked unimplemented)
+  - Has no duplicate cases
+  - Configures batching correctly, if used
+
+Example:
+  tools/validate src/unittests/ src/webgpu/
+
+Options:
+  --help          Print this message and exit.
+`);
+  process.exit(rc);
+}
+
+const args = process.argv.slice(2);
+if (args.indexOf('--help') !== -1) {
+  usage(0);
+}
+
+if (args.length < 1) {
+  usage(0);
+}
+
+for (const suiteDir of args) {
+  void crawl(suiteDir, true);
+}

Diff for: src/common/tools/validate.ts

@@ -16,7 +16,7 @@ if (!fs.existsSync(myself)) {
 
 const { version } = require('../src/common/tools/version.ts');
 
-fs.mkdirSync('./out/common/framework', { recursive: true });
+fs.mkdirSync('./out/common/internal', { recursive: true });
 // Overwrite the version.js generated by TypeScript compilation.
 fs.writeFileSync(
   './out/common/internal/version.js',

Diff for: tools/gen_version

@@ -0,0 +1,6 @@
+#!/usr/bin/env node
+
+// Validate several properties of test files and the tests in them.
+
+require('../src/common/tools/setup-ts-in-node.js');
+require('../src/common/tools/validate.ts');