commit 298ad0fff1969cf27584f5e4665def81d90b05e8 parent b23aa8a7ae3481a9fc759e7afcb2c853dfc568cd Author: Tom Marble <tmarble@info9.net> Date: Thu, 13 Nov 2025 22:03:05 +0000 Bug 1991965 - TOML linter for test manifests r=ahal Updated test-manifest-toml linter - Updated excluded paths - Improved self tests: covers fixes, missing includes - Added optional fixes including idiomatic manifest conditions Removed obsolete *.ini linters (and self tests) - tools/lint/test-manifest-alpha.yml - tools/lint/test-manifest-disable.yml - tools/lint/test-manifest-skip-if.yml Signed-off-by: Tom Marble <tmarble@info9.net> Differential Revision: https://phabricator.services.mozilla.com/D272178 Diffstat:
26 files changed, 339 insertions(+), 358 deletions(-)
diff --git a/docs/code-quality/lint/linters/test-manifest-toml.rst b/docs/code-quality/lint/linters/test-manifest-toml.rst @@ -33,3 +33,10 @@ Errors Detected * Section name not double quoted (fixable) * Disabling a path by commenting out the section * Conditional contains explicit || +* Conditional is NOT an array +* Missing include file + +Non idiomatic manifest warnings +------------------------------- +* Using ``processor`` instead of ``arch`` +* Using ``bits`` instead of ``arch`` diff --git a/python/mozbuild/mozbuild/test/backend/data/test-manifests-backend-sources/mochitest.toml b/python/mozbuild/mozbuild/test/backend/data/test-manifests-backend-sources/mochitest.toml @@ -1,6 +1,6 @@ [DEFAULT] prefs = "pref.in.ancestor=1" -["include:mochitest-common.ini"] +["include:mochitest-common.toml"] ["test_foo.js"] diff --git a/python/mozbuild/mozbuild/test/frontend/data/test-manifest-install-includes/mochitest.toml b/python/mozbuild/mozbuild/test/frontend/data/test-manifest-install-includes/mochitest.toml @@ -1,3 +1,3 @@ [DEFAULT] -["include:common.ini"] +["include:common.toml"] diff --git a/taskcluster/kinds/source-test/mozlint.yml b/taskcluster/kinds/source-test/mozlint.yml @@ -443,13 +443,12 @@ test-manifest: treeherder: symbol: misc(tm) run: - mach: lint -v -l test-manifest-alpha -l test-manifest-disable -l test-manifest-skip-if -l test-manifest-toml -f treeherder -f json:/builds/worker/mozlint.json . + mach: lint -v -l test-manifest-toml -f treeherder -f json:/builds/worker/mozlint.json . when: files-changed: - '**/*.toml' - 'python/mozlint/**' - 'tools/lint/**' - - 'tools/lint/test-disable.yml' wptlint-gecko: description: web-platform-tests linter diff --git a/testing/mozbase/manifestparser/manifestparser/toml.py b/testing/mozbase/manifestparser/manifestparser/toml.py @@ -157,7 +157,7 @@ def read_toml( new_val = new_val[0 : comment_found.span()[0]] if " = " in new_val: raise Exception( - f"Should not assign in {key} condition for {section}" + f"Should not assign in {key} list condition for {section}" ) new_vals += new_val val = new_vals @@ -194,16 +194,67 @@ def read_toml( return sections, defaults, manifest -def alphabetize_toml_str(manifest): +def idiomatic_condition(condition: str) -> str: + """converts a manifest condtiion into an idiomatic form""" + + AND: str = " && " + EQUAL: str = " == " + ops: ListStr = condition.replace("processor", "arch").split(AND) + new_ops: ListStr = [] + new_op: OptStr = None + os: OptStr = None + arch: OptStr = None + for op in ops: + if EQUAL in op: + k, v = op.split(EQUAL) + if k == "os": + os = v.strip("'\"") + elif k == "arch": + arch = v.strip("'\"") + for op in ops: + new_op = op + if EQUAL in op: + k, v = op.split(EQUAL) + if k == "bits": + if arch is None: + if v == "32": + new_op = "arch == 'x86'" + elif os is not None and os == "mac": + new_op = "arch == 'aarch64'" # assume Arm on Mac + else: + new_op = "arch == 'x86_64'" # else assume Intel + else: + new_op = None + if new_op is not None: + new_ops.append(new_op) + return AND.join(new_ops) + + +def add_unique_condition( + conds: OptConditions, condition: str, comment: str +) -> OptConditions: + """only add a condition if it is unique""" + + ignore: bool = False + for c in conds: + if _should_ignore_new_condition(c[0], condition): + ignore = True + if not ignore: + conds.append([condition, comment]) + return conds + + +def alphabetize_toml_str(manifest, fix: bool = False): """ Will take a TOMLkit manifest document (i.e. from a previous invocation of read_toml(..., document=True) and accessing the document from mp.source_documents[filename]) and return it as a string in sorted order by section (i.e. test file name, taking bug ids into consideration). + If fix then fix non-idiomatic conditions """ - from tomlkit import document, dumps, table - from tomlkit.items import Table + from tomlkit import array, document, dumps, table + from tomlkit.items import Comment, String, Whitespace preamble = "" new_manifest = document() @@ -233,6 +284,61 @@ def alphabetize_toml_str(manifest): if section is None: section = str(k).strip("'\"") sections[section] = v + if fix: + mp_array: Array = array() + conds: OptConditions = [] + first: OptStr = None # first skip-if condition + first_comment: str = "" # first skip-if comment + e_cond: OptStr = None # existing skip-if condition + e_comment: str = "" # existing skip-if comment + + keyvals: dict = sections[section] + for cond, skip_if in keyvals.items(): + if not cond.endswith("-if"): + continue + + # handle the first condition uniquely to perserve whitespace + if len(skip_if) == 1: + for e in skip_if._iter_items(): + if first is None: + if not isinstance(e, Whitespace): + first = e.as_string().strip('"') + else: + c: str = e.as_string() + if c != ",": + first_comment += c + if skip_if.trivia is not None: + first_comment += skip_if.trivia.comment + if first is not None: + if first_comment: + first_comment = _simplify_comment(first_comment) + e_cond = idiomatic_condition(first) + e_comment = first_comment + + # loop over all skip-if conditions + for e in skip_if._iter_items(): + if isinstance(e, String): + if e_cond is not None: + conds = add_unique_condition(conds, e_cond, e_comment) + e_cond = None + e_comment = "" + if len(e) > 0: + e_cond = e.as_string().strip('"') + if e_cond == first: + e_cond = None # don't repeat first + else: + e_cond = idiomatic_condition(e_cond) + elif isinstance(e, Comment): + e_comment = _simplify_comment(e.as_string()) + if e_cond is not None: + conds = add_unique_condition(conds, e_cond, e_comment) + + # Save updated conditions in order + conds.sort() + for c in conds: + mp_array.add_line(c[0], indent=" ", comment=c[1]) + mp_array.add_line("", indent="") # fixed in write_toml_str + sections[section][cond] = mp_array if not first_section: new_manifest.add(DEFAULT_SECTION, table()) @@ -241,6 +347,7 @@ def alphabetize_toml_str(manifest): new_manifest.add(section, sections[section]) manifest_str = dumps(new_manifest) + # tomlkit fixups manifest_str = preamble + manifest_str.replace('"",]', "]") while manifest_str.endswith("\n\n"): @@ -451,7 +558,7 @@ def add_skip_if( first: OptStr = None first_comment: str = "" skip_if: OptArray = None - ignore_condition: bool = False # this condition should not be added + ignore_cond: bool = False # this condition should not be added if "skip-if" in keyvals: skip_if = keyvals["skip-if"] if len(skip_if) == 1: @@ -480,16 +587,16 @@ def add_skip_if( else: # We store the conditions in a regular python array so we can sort them before # dumping them in the TOML - conditions_array: OptConditions = [] + conds: OptConditions = [] if first is not None: if _should_ignore_new_condition(first, condition): - ignore_condition = True + ignore_cond = True if first_comment: first_comment = _simplify_comment(first_comment) if _should_keep_existing_condition(first, condition): - conditions_array.append([first, first_comment]) + conds.append([first, first_comment]) if ( - not ignore_condition + not ignore_cond and mode == Mode.CARRYOVER and carry.is_carryover(first, condition) ): @@ -498,43 +605,43 @@ def add_skip_if( elif bug_reference is None and create_bug_lambda is None: bug_reference = first_comment if len(skip_if) > 1: - e_condition = None + e_cond = None e_comment = None for e in skip_if._iter_items(): if isinstance(e, String): - if e_condition is not None: - if _should_keep_existing_condition(e_condition, condition): - conditions_array.append([e_condition, e_comment]) + if e_cond is not None: + if _should_keep_existing_condition(e_cond, condition): + conds.append([e_cond, e_comment]) if ( - not ignore_condition + not ignore_cond and mode == Mode.CARRYOVER - and carry.is_carryover(e_condition, condition) + and carry.is_carryover(e_cond, condition) ): carryover = True bug_reference = e_comment elif bug_reference is None and create_bug_lambda is None: bug_reference = e_comment e_comment = None - e_condition = None + e_cond = None if len(e) > 0: - e_condition = e.as_string().strip('"') - if _should_ignore_new_condition(e_condition, condition): - ignore_condition = True + e_cond = e.as_string().strip('"') + if _should_ignore_new_condition(e_cond, condition): + ignore_cond = True elif isinstance(e, Comment): e_comment = _simplify_comment(e.as_string()) - if e_condition is not None: - if _should_keep_existing_condition(e_condition, condition): - conditions_array.append([e_condition, e_comment]) + if e_cond is not None: + if _should_keep_existing_condition(e_cond, condition): + conds.append([e_cond, e_comment]) if ( - not ignore_condition + not ignore_cond and mode == Mode.CARRYOVER - and carry.is_carryover(e_condition, condition) + and carry.is_carryover(e_cond, condition) ): carryover = True bug_reference = e_comment elif bug_reference is None and create_bug_lambda is None: bug_reference = e_comment - if ignore_condition: + if ignore_cond: carryover = False bug_reference = None else: @@ -542,9 +649,9 @@ def add_skip_if( bug = create_bug_lambda() if bug is not None: bug_reference = f"Bug {bug.id}" - conditions_array.append([condition, bug_reference]) - conditions_array.sort() - for c in conditions_array: + conds.append([condition, bug_reference]) + conds.sort() + for c in conds: mp_array.add_line(c[0], indent=" ", comment=c[1]) mp_array.add_line("", indent="") # fixed in write_toml_str skip_if = {"skip-if": mp_array} @@ -553,7 +660,7 @@ def add_skip_if( return (additional_comment, carryover, bug_reference) -def _should_remove_condition( +def _should_remove_cond( condition: str, os_name: OptStr = None, os_version: OptStr = None, @@ -592,7 +699,7 @@ def remove_skip_if( for item in condition_array._iter_items(): if isinstance(item, String): if condition is not None: - if not _should_remove_condition( + if not _should_remove_cond( condition, os_name, os_version, processor ): conditions_to_add.append((condition, comment)) @@ -605,7 +712,7 @@ def remove_skip_if( elif isinstance(item, Comment): comment = _simplify_comment(item.as_string()) if condition is not None: - if not _should_remove_condition( + if not _should_remove_cond( condition, os_name, os_version, processor ): conditions_to_add.append((condition, comment)) @@ -662,10 +769,10 @@ def replace_tbd_skip_if( ) skip_if: Array = keyvals["skip-if"] mp_array: Array = array() - conditions_array: OptConditions = [] + conds: OptConditions = [] first: OptStr = None # first skip-if condition first_comment: str = "" # first skip-if comment - e_condition: OptStr = None # existing skip-if condition + e_cond: OptStr = None # existing skip-if condition e_comment: str = "" # existing skip-if comment # handle the first condition uniquely to properly perserve whitespace @@ -687,32 +794,32 @@ def replace_tbd_skip_if( i: int = max(first_comment.find(BUG_TBD), 0) first_comment = f"{' ' * i}Bug {bugid}" updated = True - e_condition = first + e_cond = first e_comment = first_comment # loop over all skip-if conditions to find BUG_TBD for e in skip_if._iter_items(): if isinstance(e, String): - if e_condition is not None: - conditions_array.append([e_condition, e_comment]) - e_condition = None + if e_cond is not None: + conds.append([e_cond, e_comment]) + e_cond = None e_comment = "" if len(e) > 0: - e_condition = e.as_string().strip('"') - if e_condition == first: - e_condition = None # don't repeat first + e_cond = e.as_string().strip('"') + if e_cond == first: + e_cond = None # don't repeat first elif isinstance(e, Comment): e_comment = _simplify_comment(e.as_string()) - if e_condition == condition and e_comment.endswith(BUG_TBD): + if e_cond == condition and e_comment.endswith(BUG_TBD): i: int = max(e_comment.find(BUG_TBD), 0) e_comment = f"{' ' * i}Bug {bugid}" updated = True - if e_condition is not None: - conditions_array.append([e_condition, e_comment]) + if e_cond is not None: + conds.append([e_cond, e_comment]) # Update TOML document for the test with the updated skip-if comment - conditions_array.sort() - for c in conditions_array: + conds.sort() + for c in conds: mp_array.add_line(c[0], indent=" ", comment=c[1]) mp_array.add_line("", indent="") # fixed in write_toml_str skip_if = {"skip-if": mp_array} diff --git a/testing/mozbase/manifestparser/tests/test_manifestparser.py b/testing/mozbase/manifestparser/tests/test_manifestparser.py @@ -540,7 +540,7 @@ yellow = submarine parser = ManifestParser(use_toml=True) manifest = os.path.join(here, "broken-skip-if.toml") with self.assertRaisesRegex( - Exception, "Should not assign in skip-if condition for DEFAULT" + Exception, "Should not assign in skip-if list condition for DEFAULT" ): parser.read(manifest) diff --git a/tools/lint/test-manifest-alpha.yml b/tools/lint/test-manifest-alpha.yml @@ -1,19 +0,0 @@ ---- -test-manifest-alpha: - description: Mochitest manifest tests should be in alphabetical order. - exclude: - - "**/application.ini" - - "**/l10n.ini" - - "**/xpcshell.ini" - - "**/python.ini" - - "**/manifest.ini" - - dom/canvas/test/webgl-conf/mochitest-errata.toml - - python/mozbuild/mozbuild/test/backend/data - - testing/mozbase/manifestparser/tests - - testing/web-platform - - xpcom/tests/unit/data - extensions: ['ini'] - type: external - payload: test-manifest-alpha:lint - support-files: - - 'tools/lint/test-manifest-alpha/error-level-manifests.yml' diff --git a/tools/lint/test-manifest-alpha/__init__.py b/tools/lint/test-manifest-alpha/__init__.py @@ -1,77 +0,0 @@ -# This Source Code Form is subject to the terms of the Mozilla Public -# License, v. 2.0. If a copy of the MPL was not distributed with this -# file, You can obtain one at http://mozilla.org/MPL/2.0/. - -import difflib -import os -import sys - -import yaml -from manifestparser import TestManifest -from mozlint import result -from mozlint.pathutils import expand_exclusions -from mozpack import path as mozpath - -# Since this linter matches on all files with .ini extensions, we can omit -# those extensions from this allowlist, which makes it easier to match -# against variants like "mochitest-serviceworker.ini". -FILENAME_ALLOWLIST = ["mochitest", "browser", "chrome", "a11y"] - -here = os.path.abspath(os.path.dirname(__file__)) -ERROR_LEVEL_MANIFESTS_PATH = os.path.join(here, "error-level-manifests.yml") - - -def lint(paths, config, fix=None, **lintargs): - try: - with open(ERROR_LEVEL_MANIFESTS_PATH) as f: - error_level_manifests = yaml.safe_load(f) - except (OSError, ValueError) as e: - print(f"{ERROR_LEVEL_MANIFESTS_PATH}: error:\n {e}", file=sys.stderr) - sys.exit(1) - - topsrcdir = lintargs["root"] - - results = [] - file_names = list(expand_exclusions(paths, config, lintargs["root"])) - - for file_name in file_names: - name = os.path.basename(file_name) - if not any(name.startswith(allowed) for allowed in FILENAME_ALLOWLIST): - continue - - manifest = TestManifest(manifests=(file_name,), strict=False) - - test_names = [test["name"] for test in manifest.tests] - sorted_test_names = sorted(test_names) - - if test_names != sorted_test_names: - rel_file_path = mozpath.relpath(file_name, topsrcdir) - level = "warning" - - if (mozpath.normsep(rel_file_path) in error_level_manifests) or ( - any( - mozpath.match(rel_file_path, e) - for e in error_level_manifests - if "*" in e - ) - ): - level = "error" - - diff_instance = difflib.Differ() - diff_result = diff_instance.compare(test_names, sorted_test_names) - diff_list = list(diff_result) - - res = { - "path": rel_file_path, - "lineno": 0, - "column": 0, - "message": ( - "The mochitest test manifest is not in alphabetical order. " - "Expected ordering: \n\n%s\n\n" % "\n".join(sorted_test_names) - ), - "level": level, - "diff": "\n".join(diff_list), - } - results.append(result.from_config(config, **res)) - - return {"results": results, "fixed": 0} diff --git a/tools/lint/test-manifest-alpha/error-level-manifests.yml b/tools/lint/test-manifest-alpha/error-level-manifests.yml @@ -1,8 +0,0 @@ ---- -# This file contains a list of manifest files or directory patterns that have -# have been put into alphabetical order already. Items in this list will -# cause the test-manifest-alpha linter to use the Error level rather than -# the Warning level. - -- browser/** -- mobile/** diff --git a/tools/lint/test-manifest-disable.yml b/tools/lint/test-manifest-disable.yml @@ -1,16 +0,0 @@ ---- -no-comment-disable: - description: > - "Use 'disabled=<reason>' to disable a test instead of a - comment" - include: ['.'] - exclude: - - "**/application.ini" - - "**/l10n.ini" - - dom/canvas/test/webgl-conf/mochitest-errata.toml - - testing/mozbase/manifestparser/tests - - testing/web-platform - - xpcom/tests/unit/data - extensions: ['ini'] - type: regex - payload: ^[ \t]*(#|;)[ \t]*\[ diff --git a/tools/lint/test-manifest-skip-if.yml b/tools/lint/test-manifest-skip-if.yml @@ -1,18 +0,0 @@ ---- -multiline-skip-if: - description: Conditions joined by || should be on separate lines - hint: | - skip-if = - <condition one> # reason - <condition two> # reason - exclude: - - "**/application.ini" - - "**/l10n.ini" - - dom/canvas/test/webgl-conf/mochitest-errata.toml - - testing/mozbase/manifestparser/tests - - testing/web-platform - - xpcom/tests/unit/data - extensions: ['ini'] - level: warning - type: regex - payload: '^\s*(skip|fail)-if\s*=[^(]*\|\|' diff --git a/tools/lint/test-manifest-toml.yml b/tools/lint/test-manifest-toml.yml @@ -5,21 +5,28 @@ test-manifest-toml: - 'dom/canvas/test/webgl-conf/mochitest-errata.toml' - 'gradle/libs.versions.toml' - 'intl/icu/source/test/testdata/codepointtrie/' - - 'python/mozbuild/mozbuild/test/' - 'mobile/android/bors.toml' - 'mobile/android/fenix/bors.toml' + - 'python/mozbuild/mozbuild/test/' + - 'toolkit/components/antitracking/test/marionette/' - 'testing/cppunittest.toml' # Tries to unquote entries - bug 1953009 - - 'testing/marionette/harness/marionette_harness/tests/unit-tests.toml' + - 'testing/marionette/harness/marionette_harness/tests/' + - 'testing/mochitest/manifests/' - 'testing/mozbase/manifestparser/tests/' - - 'testing/raptor/raptor/tests/' - - 'third_party/rust/' + - 'testing/raptor/raptor/' + - 'testing/test/data/' + - 'testing/update/' + - 'tools/lint/test/files/' + - 'third_party/' - 'toolkit/components/uniffi-bindgen-gecko-js/askama.toml' - - '**/.*ruff.toml' + - '**/.ruff.toml' + - '**/ruff.toml' - '**/Cargo.toml' - '**/Cross.toml' - '**/Features.toml' - '**/ServoBindings.toml' - '**/askama.toml' + - '**/audit.toml' - '**/audits.toml' - '**/cbindgen.toml' - '**/clippy.toml' @@ -30,7 +37,10 @@ test-manifest-toml: - '**/l10n.toml' - '**/labels.toml' - '**/pyproject.toml' + - '**/python.toml' - '**/rustfmt.toml' + - '**/servo-tidy.toml' + - '**/tests/manifest.toml' - '**/uniffi.toml' extensions: ['toml'] type: external diff --git a/tools/lint/test-manifest-toml/__init__.py b/tools/lint/test-manifest-toml/__init__.py @@ -42,13 +42,14 @@ def lint(paths, config, fix=None, **lintargs): for file_name in file_names: path = mozpath.relpath(file_name, topsrcdir) - os.path.basename(file_name) + if path == ".cargo/audit.toml": + continue # special case that cannot be excluded in yml parser = TestManifest(use_toml=True, document=True) try: parser.read(file_name) - except Exception: - r = make_result(path, "The manifest is not valid TOML.", True) + except Exception as e: + r = make_result(path, f"The manifest is not valid TOML: {str(e)}", True) results.append(result.from_config(config, **r)) continue @@ -100,7 +101,7 @@ def lint(paths, config, fix=None, **lintargs): for section, keyvals in manifest.body: if section is None: continue - if not isinstance(keyvals, Table): + elif not isinstance(keyvals, Table): r = make_result( path, f"Bad assignment in preamble: {section} = {keyvals}", True ) @@ -124,9 +125,21 @@ def lint(paths, config, fix=None, **lintargs): True, ) results.append(result.from_config(config, **r)) + if e.find("bits == ") > 0: + r = make_result( + path, + "using 'bits' is not idiomatic, use 'arch' instead", + ) + results.append(result.from_config(config, **r)) + if e.find("processor == ") > 0: + r = make_result( + path, + "using 'processor' is not idiomatic, use 'arch' instead", + ) + results.append(result.from_config(config, **r)) if fix: - manifest_str = alphabetize_toml_str(manifest) + manifest_str = alphabetize_toml_str(manifest, True) fp = open(file_name, "w", encoding="utf-8", newline="\n") fp.write(manifest_str) fp.close() diff --git a/tools/lint/test/files/test-manifest-alpha/mochitest-in-order.ini b/tools/lint/test/files/test-manifest-alpha/mochitest-in-order.ini @@ -1,29 +0,0 @@ -[DEFAULT] -support-files = - click-event-helper.js - head.js - test-button-overlay.html - ../../../../dom/media/test/gizmo.mp4 - ../../../../dom/media/test/owl.mp3 - -prefs = - media.videocontrols.picture-in-picture.display-text-tracks.enabled=false - -[browser_AAA_run_first_firstTimePiPToggleEvents.js] -[browser_aaa_run_first_firstTimePiPToggleEvents.js] -[browser_backgroundTab.js] -[browser_cannotTriggerFromContent.js] -[browser_closePipPause.js] -skip-if = os == "linux" && bits == 64 && os_version == "18.04" # Bug 1569205 -[browser_closePip_pageNavigationChanges.js] -[browser_closePlayer.js] -[browser_closeTab.js] -[browser_close_unpip_focus.js] -[browser_contextMenu.js] -[browser_cornerSnapping.js] -run-if = os == "mac" -[browser_dblclickFullscreen.js] -[browser_flipIconWithRTL.js] -skip-if = - os == "linux" && ccov # Bug 1678091 - tsan # Bug 1678091 diff --git a/tools/lint/test/files/test-manifest-alpha/mochitest-mostly-in-order.ini b/tools/lint/test/files/test-manifest-alpha/mochitest-mostly-in-order.ini @@ -1,30 +0,0 @@ -[DEFAULT] -support-files = - click-event-helper.js - head.js - test-button-overlay.html - ../../../../dom/media/test/gizmo.mp4 - ../../../../dom/media/test/owl.mp3 - -prefs = - media.videocontrols.picture-in-picture.display-text-tracks.enabled=false - -[browser_AAA_run_first_firstTimePiPToggleEvents.js] -[browser_aaa_run_first_firstTimePiPToggleEvents.js] -[browser_backgroundTab.js] -[browser_cannotTriggerFromContent.js] -[browser_closePipPause.js] -skip-if = os == "linux" && bits == 64 && os_version == "18.04" # Bug 1569205 -[browser_closePip_pageNavigationChanges.js] -[browser_closePlayer.js] -[browser_closeTab.js] -[browser_close_unpip_focus.js] -[browser_contextMenu.js] -[browser_cornerSnapping.js] -run-if = os == "mac" -[browser_dblclickFullscreen.js] -[browser_flipIconWithRTL.js] -skip-if = - os == "linux" && ccov # Bug 1678091 - tsan # Bug 1678091 -[browser_a_new_test.js] diff --git a/tools/lint/test/files/test-manifest-alpha/mochitest-very-out-of-order.ini b/tools/lint/test/files/test-manifest-alpha/mochitest-very-out-of-order.ini @@ -1,29 +0,0 @@ -[DEFAULT] -support-files = - click-event-helper.js - head.js - test-button-overlay.html - ../../../../dom/media/test/gizmo.mp4 - ../../../../dom/media/test/owl.mp3 - -prefs = - media.videocontrols.picture-in-picture.display-text-tracks.enabled=false - -[browser_contextMenu.js] -[browser_aaa_run_first_firstTimePiPToggleEvents.js] -[browser_AAA_run_first_firstTimePiPToggleEvents.js] -[browser_backgroundTab.js] -[browser_cannotTriggerFromContent.js] -[browser_close_unpip_focus.js] -[browser_closePip_pageNavigationChanges.js] -[browser_closePipPause.js] -skip-if = os == "linux" && bits == 64 && os_version == "18.04" # Bug 1569205 -[browser_cornerSnapping.js] -run-if = os == "mac" -[browser_closePlayer.js] -[browser_closeTab.js] -[browser_dblclickFullscreen.js] -[browser_flipIconWithRTL.js] -skip-if = - os == "linux" && ccov # Bug 1678091 - tsan # Bug 1678091 diff --git a/tools/lint/test/files/test-manifest-alpha/other-ini-very-out-of-order.ini b/tools/lint/test/files/test-manifest-alpha/other-ini-very-out-of-order.ini @@ -1,29 +0,0 @@ -[DEFAULT] -support-files = - click-event-helper.js - head.js - test-button-overlay.html - ../../../../dom/media/test/gizmo.mp4 - ../../../../dom/media/test/owl.mp3 - -prefs = - media.videocontrols.picture-in-picture.display-text-tracks.enabled=false - -[browser_contextMenu.js] -[browser_aaa_run_first_firstTimePiPToggleEvents.js] -[browser_AAA_run_first_firstTimePiPToggleEvents.js] -[browser_backgroundTab.js] -[browser_cannotTriggerFromContent.js] -[browser_close_unpip_focus.js] -[browser_closePip_pageNavigationChanges.js] -[browser_closePipPause.js] -skip-if = os == "linux" && bits == 64 && os_version == "18.04" # Bug 1569205 -[browser_cornerSnapping.js] -run-if = os == "mac" -[browser_closePlayer.js] -[browser_closeTab.js] -[browser_dblclickFullscreen.js] -[browser_flipIconWithRTL.js] -skip-if = - os == "linux" && ccov # Bug 1678091 - tsan # Bug 1678091 diff --git a/tools/lint/test/files/test-manifest-toml/assignment-in-preamble.toml b/tools/lint/test/files/test-manifest-toml/assignment-in-preamble.toml @@ -0,0 +1,6 @@ +# Assignment in preamble +https_first_disabled = true + +[DEFAULT] + +["bbb.js"] diff --git a/tools/lint/test/files/test-manifest-toml/missing-include.toml b/tools/lint/test/files/test-manifest-toml/missing-include.toml @@ -0,0 +1,3 @@ +[DEFAULT] + +["include:non-existent.toml"] diff --git a/tools/lint/test/files/test-manifest-toml/non-double-quote-sections-fix.toml b/tools/lint/test/files/test-manifest-toml/non-double-quote-sections-fix.toml @@ -0,0 +1,6 @@ +[DEFAULT] + +["aaa.js"] +# Every non DEFAULT section must be double quoted + +["bbb.js"] diff --git a/tools/lint/test/files/test-manifest-toml/non-idiomatic-fix.toml b/tools/lint/test/files/test-manifest-toml/non-idiomatic-fix.toml @@ -0,0 +1,10 @@ +# Non-idiomatic manifest +[DEFAULT] + +["aaa.js"] +skip-if = [ + "os == 'mac' && os_version == '11.20' && arch == 'aarch64'", # 1 + "os == 'mac' && os_version == '15.30' && arch == 'aarch64'", # 2 + "os == 'mac' && os_version == '15.30' && arch == 'x86_64'", # 4 + "win11_2009 && arch == 'x86'", # 5 +] diff --git a/tools/lint/test/files/test-manifest-toml/non-idiomatic.toml b/tools/lint/test/files/test-manifest-toml/non-idiomatic.toml @@ -0,0 +1,11 @@ +# Non-idiomatic manifest +[DEFAULT] + +["aaa.js"] +skip-if = [ + "os == 'mac' && os_version == '11.20' && processor == 'aarch64'", # 1 + "os == 'mac' && os_version == '15.30' && bits == 64", # 2 + "os == 'mac' && os_version == '15.30' && processor == 'aarch64' && bits == 64", # 3 + "os == 'mac' && os_version == '15.30' && processor == 'x86_64'", # 4 + "win11_2009 && bits == 32", # 5 +] diff --git a/tools/lint/test/files/test-manifest-toml/unsorted-fix.toml b/tools/lint/test/files/test-manifest-toml/unsorted-fix.toml @@ -0,0 +1,10 @@ +[DEFAULT] +# unsorted sections + +["aaa.js"] + +["bug_2.js"] + +["bug_10.js"] + +["ccc.js"] diff --git a/tools/lint/test/python.toml b/tools/lint/test/python.toml @@ -42,8 +42,6 @@ skip-if = ["os == 'win'"] ["test_lintpref.py"] -["test_manifest_alpha.py"] - ["test_manifest_toml.py"] ["test_node_licenses.py"] diff --git a/tools/lint/test/test_manifest_alpha.py b/tools/lint/test/test_manifest_alpha.py @@ -1,33 +0,0 @@ -# This Source Code Form is subject to the terms of the Mozilla Public -# License, v. 2.0. If a copy of the MPL was not distributed with this -# file, You can obtain one at http://mozilla.org/MPL/2.0/. -import mozunit - -LINTER = "test-manifest-alpha" - - -def test_very_out_of_order(lint, paths): - results = lint(paths("mochitest-very-out-of-order.ini")) - assert len(results) == 1 - assert results[0].diff - - -def test_in_order(lint, paths): - results = lint(paths("mochitest-in-order.ini")) - assert len(results) == 0 - - -def test_mostly_in_order(lint, paths): - results = lint(paths("mochitest-mostly-in-order.ini")) - assert len(results) == 1 - assert results[0].diff - - -def test_other_ini_very_out_of_order(lint, paths): - """Test that an .ini file outside of the allowlist is ignored.""" - results = lint(paths("other-ini-very-out-of-order.ini")) - assert len(results) == 0 - - -if __name__ == "__main__": - mozunit.main() diff --git a/tools/lint/test/test_manifest_toml.py b/tools/lint/test/test_manifest_toml.py @@ -1,11 +1,17 @@ # This Source Code Form is subject to the terms of the Mozilla Public # License, v. 2.0. If a copy of the MPL was not distributed with this # file, You can obtain one at http://mozilla.org/MPL/2.0/. + +from pathlib import Path + import mozunit LINTER = "test-manifest-toml" fixed = 0 +ERROR = "error" +WARNING = "warning" + def test_valid(lint, paths): results = lint(paths("valid.toml")) @@ -15,63 +21,146 @@ def test_valid(lint, paths): def test_invalid(lint, paths): results = lint(paths("invalid.toml")) assert len(results) == 1 - assert results[0].message == "The manifest is not valid TOML." + assert results[0].message.startswith("The manifest is not valid TOML") + assert results[0].level == ERROR + + +def test_assignment_in_preamble(lint, paths): + # NOTE: The tomlkit parser will throw an error for this legal TOML + # so our linter never gets to find the preamble assignment + results = lint(paths("assignment-in-preamble.toml")) + assert len(results) == 1 + assert ( + results[0].message + == "The manifest is not valid TOML: 'bool' object has no attribute 'keys'" + ) + assert results[0].level == ERROR def test_no_default(lint, paths): """Test verifying [DEFAULT] section.""" + results = lint(paths("no-default.toml")) assert len(results) == 1 assert results[0].message == "The manifest does not start with a [DEFAULT] section." + assert results[0].level == WARNING def test_no_default_fix(lint, paths, create_temp_file): """Test fixing missing [DEFAULT] section.""" + contents = "# this Manifest has no DEFAULT section\n" path = create_temp_file(contents, "no-default.toml") results = lint([path], fix=True) assert len(results) == 1 assert results[0].message == "The manifest does not start with a [DEFAULT] section." + assert results[0].level == WARNING assert fixed == 1 + expected = contents + "[DEFAULT]\n" + assert Path(path).read_text() == expected -def test_non_double_quote_sections(lint, paths): - """Test verifying [DEFAULT] section.""" - results = lint(paths("non-double-quote-sections.toml")) +def test_non_double_quote_sections_fix(lint, paths, create_temp_file): + """Test and fix sections that do not have double quotes""" + + basename = "non-double-quote-sections" + orig, fix = paths(f"{basename}.toml", f"{basename}-fix.toml") + original = Path(orig).read_text() + expected = Path(fix).read_text() + path = create_temp_file(original, f"{basename}.toml") + results = lint([path], fix=True) assert len(results) == 2 - assert results[0].message.startswith("The section name must be double quoted:") + assert results[0].message == "The section name must be double quoted: ['bbb.js']" + assert results[0].level == WARNING + assert results[1].message == "The section name must be double quoted: [aaa.js]" + assert results[1].level == WARNING + assert Path(path).read_text() == expected + +def test_unsorted_fix(lint, paths, create_temp_file): + """Test and fix sections in alpha order.""" -def test_unsorted(lint, paths): - """Test sections in alpha order.""" - results = lint(paths("unsorted.toml")) + basename = "unsorted" + orig, fix = paths(f"{basename}.toml", f"{basename}-fix.toml") + original = Path(orig).read_text() + expected = Path(fix).read_text() + path = create_temp_file(original, f"{basename}.toml") + results = lint([path], fix=True) assert len(results) == 1 assert results[0].message == "The manifest sections are not in alphabetical order." + assert results[0].level == WARNING + assert Path(path).read_text() == expected def test_comment_section(lint, paths): """Test for commented sections.""" + results = lint(paths("comment-section.toml")) assert len(results) == 2 assert results[0].message.startswith( "Use 'disabled = \"<reason>\"' to disable a test instead of a comment:" ) + assert results[0].level == ERROR def test_skip_if_not_array(lint, paths): """Test for non-array skip-if value.""" + results = lint(paths("skip-if-not-array.toml")) assert len(results) == 1 assert results[0].message.startswith("Value for conditional must be an array:") + assert results[0].level == ERROR def test_skip_if_explicit_or(lint, paths): """Test for explicit || in skip-if.""" + results = lint(paths("skip-if-explicit-or.toml")) assert len(results) == 1 assert results[0].message.startswith( "Value for conditional must not include explicit ||, instead put on multiple lines:" ) + assert results[0].level == ERROR + + +def test_missing_include(lint, paths): + """Test for missing include""" + + results = lint(paths("missing-include.toml")) + assert len(results) == 1 + assert "non-existent.toml' does not exist" in results[0].message + assert results[0].level == ERROR + + +def test_non_idiomatic_fix(lint, paths, create_temp_file): + """Test and fix non-idiomatic conditions.""" + + basename = "non-idiomatic" + orig, fix = paths(f"{basename}.toml", f"{basename}-fix.toml") + original = Path(orig).read_text() + expected = Path(fix).read_text() + path = create_temp_file(original, f"{basename}.toml") + results = lint([path], fix=True) + assert len(results) == 6 + assert results[0].message == "using 'bits' is not idiomatic, use 'arch' instead" + assert results[0].level == WARNING + assert results[1].message == "using 'bits' is not idiomatic, use 'arch' instead" + assert results[1].level == WARNING + assert results[2].message == "using 'bits' is not idiomatic, use 'arch' instead" + assert results[2].level == WARNING + assert ( + results[3].message == "using 'processor' is not idiomatic, use 'arch' instead" + ) + assert results[3].level == WARNING + assert ( + results[4].message == "using 'processor' is not idiomatic, use 'arch' instead" + ) + assert results[4].level == WARNING + assert ( + results[5].message == "using 'processor' is not idiomatic, use 'arch' instead" + ) + assert results[5].level == WARNING + assert Path(path).read_text() == expected if __name__ == "__main__":