Introduce automated validation for test format (#994)

This script is intended to identify common test file formatting errors
prior to their acceptance into the project. It is designed to support
future extensions for additional validation rules.

.travis.yml

@@ -1,8 +1,9 @@
 language: python
 install: pip install --requirement tools/generation/requirements.txt
 script:
-  - echo The test generation tool should be working.
+  - ./tools/scripts/ci_build.sh
   - ./tools/generation/test/run.py
-  - sh ./tools/scripts/ci.sh
+  - ./tools/lint/test/run.py
+  - ./tools/scripts/ci_lint.sh
 after_success:
-  - sh ./tools/scripts/deploy.sh
+  - ./tools/scripts/deploy.sh

CONTRIBUTING.md

@@ -249,6 +249,19 @@ p.then(function () {
 
 As above, exceptions that are thrown from a `then` clause are passed to a later `$DONE` function and reported asynchronously.
 
+## Linting
+
+Some of the expectations documented here are enforced via a "linting" script. This script is used to validate patches automatically at submission time, but it may also be invoked locally via the following command:
+
+    python tools/lint/lint.py --whitelist lint.whitelist [paths to tests]
+
+...where `[paths to tests]` is a list of one or more paths to test files or directories containing test files.
+
+In some cases, it may be necessary for a test to intentionally violate the rules enforced by the linting tool. Such violations can be allowed by including the path of the test(s) in the `lint.whitelist` file. Each path must appear on a dedicated line in that file, and a space-separated list of rules to ignore must follow each path. Lines beginning with the pound sign (`#`) will be ignored. For example:
+
+    # This file documents authorship information and is not itself a test
+    test/built-ins/Simd/AUTHORS FRONTMATTER LICENSE
+
 ## Procedurally-generated tests
 
 Some language features are expressed through a number of distinct syntactic forms. Test262 maintains these tests as a set of "test cases" and "test templates" in order to ensure equivalent coverage across all forms. The sub-directories within the `src/` directory describe the various language features that benefit from this approach.

lint.whitelist

@@ -0,0 +1,2 @@
+# This file documents authorship information and is not itself a test
+test/built-ins/Simd/AUTHORS FRONTMATTER LICENSE

test/built-ins/Object/assign/Override.js

@@ -8,13 +8,13 @@ es6id:  19.1.2.1.5.c
 
 //"a" will be an property of the final object and the value should be 1
 var target = {a:1};
-/*---
+/*
 "1a2c3" have own enumerable properties, so it Should be wrapped to objects;
 {b:6} is an object,should be assigned to final object.
 undefined and null should be ignored;
 125 is a number,it cannot has own enumerable properties;
 {a:"c"},{a:5} will override property a, the value should be 5.
----*/
+*/
 var result = Object.assign(target,"1a2c3",{a:"c"},undefined,{b:6},null,125,{a:5});
 
 assert.sameValue(Object.keys(result).length, 7 , "The length should be 7 in the final object.");

tools/lint/lib/check.py

@@ -0,0 +1,6 @@
+class Check(object):
+    '''Base class for  defining linting checks.'''
+    ID = None
+
+    def run(self, name, meta, source):
+        return True

tools/lint/lib/checks/frontmatter.py

@@ -0,0 +1,42 @@
+from ..check import Check
+
+_REQUIRED_FIELDS = set(['description'])
+_OPTIONAL_FIELDS = set([
+    'author', 'es5id', 'es6id', 'esid', 'features', 'flags', 'includes',
+    'info', 'negative', 'timeout'
+])
+_VALID_FIELDS = _REQUIRED_FIELDS | _OPTIONAL_FIELDS
+
+class CheckFrontmatter(Check):
+    '''Ensure tests have the expected YAML-formatted metadata.'''
+    ID = 'FRONTMATTER'
+
+    def run(self, name, meta, source):
+        if name.endswith('_FIXTURE.js'):
+            if meta is not None:
+                return '"Fixture" files cannot specify metadata'
+            return
+
+        if meta is None:
+            return 'No valid YAML-formatted frontmatter'
+
+        fields = set(meta.keys())
+
+        missing = _REQUIRED_FIELDS - fields
+        if len(missing) > 0:
+            return 'Required fields missing: %s' % ', '.join(list(missing))
+
+        unrecognized = fields - _VALID_FIELDS
+        if len(unrecognized) > 0:
+            return 'Unrecognized fields: %s' % ', '.join(list(unrecognized))
+
+        if 'negative' in meta:
+            negative = meta['negative']
+            if not isinstance(negative, dict):
+                return '"negative" must be a dictionary with fields "type" and "phase"'
+
+            if not 'type' in negative:
+                return '"negative" must specify a "type" field'
+
+            if not 'phase' in negative:
+                return '"negative" must specify a "phase" field'

tools/lint/lib/checks/license.py

@@ -0,0 +1,43 @@
+import re
+
+from ..check import Check
+
+_MIN_YEAR = 2009
+_MAX_YEAR = 2030
+
+_LICENSE_PATTERN = re.compile(
+    r'\/\/ Copyright( \([cC]\))? (\w+) .+\. {1,2}All rights reserved\.[\r\n]{1,2}' +
+    r'(' +
+        r'\/\/ (' +
+            r'This code is governed by the( BSD)? license found in the LICENSE file\.' +
+            r'|' +
+            r'See LICENSE for details' +
+        r')' +
+        r'|' +
+        r'\/\/ Use of this source code is governed by a BSD-style license that can be[\r\n]{1,2}' +
+        r'\/\/ found in the LICENSE file\.' +
+        r'|' +
+        r'\/\/ See LICENSE or https://github\.com/tc39/test262/blob/master/LICENSE' +
+    r')', re.IGNORECASE)
+
+class CheckLicense(Check):
+    '''Ensure tests declare valid license information.'''
+    ID = 'LICENSE'
+
+    def run(self, name, meta, source):
+        if meta and 'flags' in meta and 'generated' in meta['flags']:
+            return
+
+        match = _LICENSE_PATTERN.search(source)
+
+        if not match:
+            return 'No license information found.'
+
+        year_str = match.group(2)
+        try:
+            year = int(year_str)
+
+            if year < _MIN_YEAR or year > _MAX_YEAR:
+                raise ValueError()
+        except ValueError:
+            return 'Invalid year: %s' % year_str

tools/lint/lib/collect_files.py

@@ -0,0 +1,20 @@
+import os
+
+def collect_files(path):
+    '''Given a path to a file, yield that path. Given a path to a directory,
+    yield the path of all files within that directory recursively, omitting any
+    that begin with a period (.) character.'''
+
+    if os.path.isfile(path):
+        yield path
+        return
+
+    if not os.path.isdir(path):
+        raise ValueError('Not found: "%s"' % path)
+
+    for root, dirs, file_names in os.walk(path):
+        for file_name in file_names:
+            if file_name.startswith('.'):
+                continue
+
+            yield os.path.join(root, file_name)

tools/lint/lib/eprint.py

@@ -0,0 +1,5 @@
+from __future__ import print_function
+import sys
+
+def eprint(*args, **kwargs):
+    print(*args, file=sys.stderr, **kwargs)

tools/lint/lib/frontmatter.py

@@ -0,0 +1,16 @@
+import re
+import yaml
+
+def parse(src):
+    '''Parse the YAML-formatted metadata found in a given string of source
+    code. Tolerate missing or invalid metadata; those conditions are handled by
+    a dedicated "Check" instance.'''
+
+    match = re.search(r'/\*---(.*)---\*/', src, re.DOTALL)
+    if not match:
+        return None
+
+    try:
+        return yaml.load(match.group(1))
+    except (yaml.scanner.ScannerError, yaml.parser.ParserError):
+        return None

tools/lint/lib/whitelist.py

@@ -0,0 +1,24 @@
+def parse(handle):
+    '''Parse the contents of the provided file descriptor as a linting
+    whitelist file. Return a dictionary whose keys are test file names and
+    whose values are Python sets of "Check" ID strings.'''
+
+    whitelist = dict()
+
+    for line in handle:
+        if line.startswith('#'):
+            continue
+
+        parts = line.split()
+        file_name = parts[0]
+        check_names = set(parts[1:])
+
+        assert file_name not in whitelist, (
+            'Whitelist should have a single entry for each file')
+
+        assert len(check_names) > 0, (
+            'Each whitelist entry should specify at least on check')
+
+        whitelist[file_name] = check_names
+
+    return whitelist

tools/lint/lint.py

@@ -0,0 +1,74 @@
+#!/usr/bin/env python
+# Copyright (C) 2017 Mike Pennisi. All rights reserved.
+# This code is governed by the BSD license found in the LICENSE file.
+
+import argparse
+import sys
+
+from lib.collect_files import collect_files
+from lib.checks.frontmatter import CheckFrontmatter
+from lib.checks.license import CheckLicense
+from lib.eprint import eprint
+import lib.frontmatter
+import lib.whitelist
+
+parser = argparse.ArgumentParser(description='Test262 linting tool')
+parser.add_argument('--whitelist',
+        type=argparse.FileType('r'),
+        help='file containing expected linting errors')
+parser.add_argument('path',
+        nargs='+',
+        help='file name or directory of files to lint')
+
+checks = [CheckFrontmatter(), CheckLicense()]
+
+def lint(file_names):
+    errors = dict()
+
+    for file_name in file_names:
+        with open(file_name, 'r') as f:
+            content = f.read()
+        meta = lib.frontmatter.parse(content)
+        for check in checks:
+            error = check.run(file_name, meta, content)
+
+            if error is not None:
+                if file_name not in errors:
+                    errors[file_name] = dict()
+                errors[file_name][check.ID] = error
+
+    return errors
+
+if __name__ == '__main__':
+    args = parser.parse_args()
+    if args.whitelist:
+        whitelist = lib.whitelist.parse(args.whitelist)
+    else:
+        whitelist = dict()
+
+    files = [path for _path in args.path for path in collect_files(_path)]
+    file_count = len(files)
+    print 'Linting %s file%s.' % (file_count, 's' if file_count != 1 else '')
+
+    all_errors = lint(files)
+    unexpected_errors = dict(all_errors)
+
+    for file_name, failures in all_errors.iteritems():
+        if file_name not in whitelist:
+            continue
+        if set(failures.keys()) == whitelist[file_name]:
+            del unexpected_errors[file_name]
+
+    error_count = len(unexpected_errors)
+    s = 's' if error_count != 1 else ''
+
+    print 'Linting complete. %s error%s found.' % (error_count, s)
+
+    if error_count == 0:
+        sys.exit(0)
+
+    for file_name, failures in unexpected_errors.iteritems():
+        for ID, message in failures.iteritems():
+            eprint('%s: %s - %s' % (file_name, ID, message))
+
+    sys.exit(1)

tools/lint/requirements.txt

@@ -0,0 +1 @@
+PyYAML==3.11

tools/lint/test/fixtures/frontmatter_invalid_yaml.js

@@ -0,0 +1,15 @@
+FRONTMATTER
+^ expected errors | v input
+// Copyright (C) 2017 Mike Pennisi. All rights reserved.
+// This code is governed by the BSD license found in the LICENSE file.
+/*---
+esid: sec-assignment-operators-static-semantics-early-errors
+es6id: 12.14.1
+description: Applied to a "covered" YieldExpression
+info: This is some information
+features: [generators
+---*/
+
+function* g() {
+  yield 23;
+}

tools/lint/test/fixtures/frontmatter_missing_desc.js

@@ -0,0 +1,13 @@
+FRONTMATTER
+^ expected errors | v input
+// Copyright (C) 2017 Mike Pennisi. All rights reserved.
+// This code is governed by the BSD license found in the LICENSE file.
+/*---
+esid: sec-assignment-operators-static-semantics-early-errors
+es6id: 12.14.1
+info: This is some information
+---*/
+
+function* g() {
+  yield 23;
+}

tools/lint/test/fixtures/frontmatter_module_FIXTURE.js

@@ -0,0 +1,7 @@
+^ expected errors | v input
+// Copyright (C) 2017 Mike Pennisi. All rights reserved.
+// This code is governed by the BSD license found in the LICENSE file.
+
+function* g() {
+  yield 23;
+}

tools/lint/test/fixtures/frontmatter_module_with_meta_FIXTURE.js

@@ -0,0 +1,15 @@
+FRONTMATTER
+^ expected errors | v input
+// Copyright (C) 2017 Mike Pennisi. All rights reserved.
+// This code is governed by the BSD license found in the LICENSE file.
+/*---
+esid: sec-assignment-operators-static-semantics-early-errors
+es6id: 12.14.1
+description: Applied to a "covered" YieldExpression
+info: This is some information
+features: [generators]
+---*/
+
+function* g() {
+  yield 23;
+}

tools/lint/test/fixtures/frontmatter_negative_missing_phase.js

@@ -0,0 +1,12 @@
+FRONTMATTER
+^ expected errors | v input
+// Copyright (C) 2017 Mike Pennisi. All rights reserved.
+// This code is governed by the BSD license found in the LICENSE file.
+/*---
+esid: sec-assignment-operators-static-semantics-early-errors
+description: Minimal test
+negative:
+  type: SyntaxError
+---*/
+
+!!!

tools/lint/test/fixtures/frontmatter_negative_missing_type.js

@@ -0,0 +1,12 @@
+FRONTMATTER
+^ expected errors | v input
+// Copyright (C) 2017 Mike Pennisi. All rights reserved.
+// This code is governed by the BSD license found in the LICENSE file.
+/*---
+esid: sec-assignment-operators-static-semantics-early-errors
+description: Minimal test
+negative:
+  phase: early
+---*/
+
+!!!

tools/lint/test/fixtures/frontmatter_negative_string.js

@@ -0,0 +1,11 @@
+FRONTMATTER
+^ expected errors | v input
+// Copyright (C) 2017 Mike Pennisi. All rights reserved.
+// This code is governed by the BSD license found in the LICENSE file.
+/*---
+esid: sec-assignment-operators-static-semantics-early-errors
+description: Minimal test
+negative: SyntaxError
+---*/
+
+!!!

tools/lint/test/fixtures/frontmatter_negative_valid.js

@@ -0,0 +1,12 @@
+^ expected errors | v input
+// Copyright (C) 2017 Mike Pennisi. All rights reserved.
+// This code is governed by the BSD license found in the LICENSE file.
+/*---
+esid: sec-assignment-operators-static-semantics-early-errors
+description: Minimal test
+negative:
+  type: SyntaxError
+  phase: early
+---*/
+
+!!!

tools/lint/test/fixtures/frontmatter_omitted.js

@@ -0,0 +1,8 @@
+FRONTMATTER
+^ expected errors | v input
+// Copyright (C) 2017 Mike Pennisi. All rights reserved.
+// This code is governed by the BSD license found in the LICENSE file.
+
+function* g() {
+  yield 23;
+}

tools/lint/test/fixtures/frontmatter_unrecognized.js

@@ -0,0 +1,15 @@
+FRONTMATTER
+^ expected errors | v input
+// Copyright (C) 2017 Mike Pennisi. All rights reserved.
+// This code is governed by the BSD license found in the LICENSE file.
+/*---
+esid: sec-assignment-operators-static-semantics-early-errors
+es6id: 12.14.1
+description: Applied to a "covered" YieldExpression
+info: This is some information
+unrecognized_attr: foo
+---*/
+
+function* g() {
+  yield 23;
+}

tools/lint/test/fixtures/license_alternate_1.js

@@ -0,0 +1,9 @@
+^ expected errors | v input
+// Copyright (c) 2017 Mike Pennisi.  All rights reserved.
+// This code is governed by the license found in the LICENSE file.
+/*---
+esid: sec-assignment-operators-static-semantics-early-errors
+description: Minimal test
+---*/
+
+void 0;

tools/lint/test/fixtures/license_alternate_2.js

@@ -0,0 +1,9 @@
+^ expected errors | v input
+// Copyright 2017 Mike Pennisi.  All rights reserved.
+// See LICENSE for details.
+/*---
+esid: sec-assignment-operators-static-semantics-early-errors
+description: Minimal test
+---*/
+
+void 0;

tools/lint/test/fixtures/license_alternate_3.js

@@ -0,0 +1,9 @@
+^ expected errors | v input
+// copyright (c) 2017 mike pennisi. all rights reserved.
+// this code is governed by the bsd license found in the license file.
+/*---
+esid: sec-assignment-operators-static-semantics-early-errors
+description: Minimal test
+---*/
+
+void 0;

tools/lint/test/fixtures/license_alternate_4,js

@@ -0,0 +1,10 @@
+^ expected errors | v input
+// Copyright (c) 2017 Mike Pennisi. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+/*---
+esid: sec-assignment-operators-static-semantics-early-errors
+description: Minimal test
+---*/
+
+void 0;

tools/lint/test/fixtures/license_alternate_5.js

@@ -0,0 +1,9 @@
+^ expected errors | v input
+// Copyright (C) 2017 Mike Pennisi. All rights reserved.
+// See LICENSE or https://github.com/tc39/test262/blob/master/LICENSE
+/*---
+esid: sec-assignment-operators-static-semantics-early-errors
+description: Minimal test
+---*/
+
+void 0;

tools/lint/test/fixtures/license_generated.js

@@ -0,0 +1,11 @@
+^ expected errors | v input
+// This file was procedurally generated from the following sources:
+// - foo
+// - bar
+/*---
+esid: sec-assignment-operators-static-semantics-early-errors
+description: Applied to a "covered" YieldExpression
+flags: [class, generated]
+---*/
+
+void 0;

tools/lint/test/fixtures/license_invalid_year.js

@@ -0,0 +1,10 @@
+LICENSE
+^ expected errors | v input
+// Copyright (C) 2199 Mike Pennisi. All rights reserved.
+// This code is governed by the BSD license found in the LICENSE file.
+/*---
+esid: sec-assignment-operators-static-semantics-early-errors
+description: Applied to a "covered" YieldExpression
+---*/
+
+void 0;

tools/lint/test/fixtures/license_missing.js

@@ -0,0 +1,8 @@
+LICENSE
+^ expected errors | v input
+/*---
+esid: sec-assignment-operators-static-semantics-early-errors
+description: Applied to a "covered" YieldExpression
+---*/
+
+void 0;

tools/lint/test/fixtures/valid_es5id.js

@@ -0,0 +1,9 @@
+^ expected errors | v input
+// Copyright (C) 2017 Mike Pennisi. All rights reserved.
+// This code is governed by the BSD license found in the LICENSE file.
+/*---
+es5id: 12.14.1
+description: Minimal test
+---*/
+
+function f() {}

tools/lint/test/fixtures/valid_es6id.js

@@ -0,0 +1,9 @@
+^ expected errors | v input
+// Copyright (C) 2017 Mike Pennisi. All rights reserved.
+// This code is governed by the BSD license found in the LICENSE file.
+/*---
+es6id: 12.14.1
+description: Minimal test
+---*/
+
+function f() {}

tools/lint/test/fixtures/valid_esid.js

@@ -0,0 +1,9 @@
+^ expected errors | v input
+// Copyright (C) 2017 Mike Pennisi. All rights reserved.
+// This code is governed by the BSD license found in the LICENSE file.
+/*---
+esid: sec-assignment-operators-static-semantics-early-errors
+description: Minimal test
+---*/
+
+function f() {}

tools/lint/test/run.py

@@ -0,0 +1,100 @@
+#!/usr/bin/env python
+# Copyright (C) 2017 Mike Pennisi. All rights reserved.
+# This code is governed by the BSD license found in the LICENSE file.
+
+import shutil, subprocess, sys, os, unittest, tempfile
+
+testDir = os.path.dirname(os.path.relpath(__file__))
+OUT_DIR = os.path.join(testDir, 'out')
+ex = os.path.join(testDir, '..', 'lint.py')
+
+class TestLinter(unittest.TestCase):
+    maxDiff = None
+
+    def fixture(self, name, content):
+        fspath = os.path.join(OUT_DIR, name)
+        with open(fspath, 'w') as f:
+            f.write(content)
+        return fspath
+
+    def lint(self, args):
+        args[:0] = [ex]
+        sp = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+        stdout, stderr = sp.communicate()
+        return dict(stdout=stdout, stderr=stderr, returncode=sp.returncode)
+
+    def setUp(self):
+        os.mkdir(OUT_DIR)
+
+    def tearDown(self):
+        shutil.rmtree(OUT_DIR, ignore_errors=True)
+
+    def test_no_file(self):
+        result = self.lint(['non-existent-file.js'])
+        self.assertNotEqual(result["returncode"], 0)
+
+    def test_whitelist_single(self):
+        test_content = ('// Copyright (C) 2017 Mike Pennisi. All rights reserved.\n' +
+            '// This code is governed by the BSD license found in the LICENSE file.')
+        test_file = self.fixture('input.js', test_content)
+        whitelist_content = test_file + ' FRONTMATTER'
+        whitelist_file = self.fixture('lint.whitelist', whitelist_content)
+
+        result = self.lint([test_file])
+
+        self.assertNotEqual(result['returncode'], 0)
+
+        result = self.lint(['--whitelist', whitelist_file, test_file])
+
+        self.assertEqual(result['returncode'], 0)
+
+    def test_whitelist_comment(self):
+        test_content = ('// Copyright (C) 2017 Mike Pennisi. All rights reserved.\n' +
+            '// This code is governed by the BSD license found in the LICENSE file.')
+        test_file = self.fixture('input.js', test_content)
+        whitelist_content = ('# One comment\n' +
+            '# Another comment\n' +
+            test_file + ' FRONTMATTER')
+        whitelist_file = self.fixture('lint.whitelist', whitelist_content)
+
+        result = self.lint([test_file])
+
+        self.assertNotEqual(result['returncode'], 0)
+
+        result = self.lint(['--whitelist', whitelist_file, test_file])
+
+        self.assertEqual(result['returncode'], 0)
+
+def create_file_test(name, fspath):
+    '''Dynamically generate a function that may be used as a test method with
+    the Python `unittest` module.'''
+
+    def test(self):
+        with open(fspath, 'r') as f:
+            contents = f.read()
+        expected, input = contents.split('^ expected errors | v input\n')
+        expected = expected.split()
+        tmp_file = self.fixture(name, input)
+        result = self.lint([tmp_file])
+        if len(expected) == 0:
+            self.assertEqual(result['returncode'], 0)
+            self.assertEqual(result['stderr'], '')
+        else:
+            self.assertNotEqual(result['returncode'], 0)
+            for err in expected:
+                self.assertIn(err, result['stderr'])
+
+    return test
+
+dirname = os.path.join(os.path.abspath(testDir), 'fixtures')
+for file_name in os.listdir(dirname):
+    full_path = os.path.join(dirname, file_name)
+    if not os.path.isfile(full_path) or file_name.startswith('.'):
+        continue
+
+    t = create_file_test(file_name, full_path)
+    t.__name__ = 'test_' + file_name
+    setattr(TestLinter, t.__name__, t)
+
+if __name__ == '__main__':
+    unittest.main()

tools/scripts/ci_lint.sh

@@ -0,0 +1,18 @@
+#!/bin/bash
+
+if [ "$TRAVIS_PULL_REQUEST" != "false" ]; then
+  paths=$(git diff --diff-filter ACMR --name-only $TRAVIS_BRANCH -- test/)
+
+  if [ "$paths" == "" ]; then
+    echo No test files added or modified. Exiting.
+    exit 0
+  fi
+
+  echo New or modified test files:
+  echo "$paths"
+
+else
+  paths="test/"
+fi
+
+./tools/lint/lint.py --whitelist lint.whitelist $paths