1
0
mirror of https://github.com/pgbackrest/pgbackrest.git synced 2026-10-23 21:28:56 +03:00

Add a linter check that every test module is defined in define.yaml.

A test module that is not declared in define.yaml is never built or run and nothing reports it, since the file simply goes unread. This was the case for common/type/c, which is added to the definition here.

The name of the file a test module lives in moves to define.py so the linter and the build cannot disagree about it.
This commit is contained in:
David Steele
2026-07-31 15:48:32 +07:00
parent 56bd6d6f53
commit b562274637
7 changed files with 102 additions and 8 deletions

View File

@@ -107,6 +107,10 @@ unit:
depend:
- common/time
# --------------------------------------------------------------------------------------------------------------------------------
- name: common/type/c
total: 2
# --------------------------------------------------------------------------------------------------------------------------------
- name: common/time
total: 3

View File

@@ -1,7 +1,7 @@
"""Code Linter.
Scans every file in the repository for content that could hide code from review and checks that StringId macros encode what they
claim to."""
Scans every file in the repository for content that could hide code from review, checks that StringId macros encode what they claim
to, and checks that every test module is declared in define.yaml."""
####################################################################################################################################
import os
@@ -9,6 +9,7 @@ import re
from command.lint.ascii import lint_ascii
from command.lint.string_id import lint_str_id
from command.test.define import TEST_MODULE_PATH, test_def_file, test_def_parse
from common.error import ToolError
from common.log import *
from common.storage import path_list_recurse
@@ -43,7 +44,15 @@ def cmd_lint(path_repo):
lib_module = {}
# File each test module declared in define.yaml lives in. A test module that is not declared is never built or run, and nothing
# else reports it since the file is simply never read.
test_module = {test_def_file(module) for module in test_def_parse(path_repo)}
for name in path_list_recurse(path_repo):
# Everything where the test modules live is a test module, so it must be declared in define.yaml
if name.startswith(TEST_MODULE_PATH) and name not in test_module:
raise ToolError("test module '%s' is not defined in test/define.yaml" % name)
# A module may appear in only one library. Python resolves a module name to the first library on the path that has it and
# ignores the rest, so a duplicate would hide the shadowed module from every tool with no indication that it had.
match = _LIB_MODULE_EXP.match(name)

View File

@@ -9,10 +9,9 @@ Anything left in the unit path that this test did not generate is removed, since
####################################################################################################################################
import os
from command.test.define import TEST_TYPE_INTEGRATION, TEST_TYPE_PERFORMANCE, TEST_TYPE_UNIT
from command.test.define import TEST_TYPE_INTEGRATION, TEST_TYPE_PERFORMANCE, TEST_TYPE_UNIT, test_def_file
from common.error import check
from common.log import *
from common.render import bld_enum
from common.storage import file_read, file_remove, file_write_differs, path_list, path_list_recurse
from common.user import group_id, group_name, user_id, user_name
@@ -442,7 +441,7 @@ class TestBuild:
# Test id
"{[C_TEST_IDX]}": "%u" % self.vm_id,
# Include test file
"{[C_TEST_INCLUDE]}": '#include "%s/test/src/module/%sTest.c"' % (path_repo_rel, bld_enum(None, module.name)),
"{[C_TEST_INCLUDE]}": '#include "%s/%s"' % (path_repo_rel, test_def_file(module)),
# Test list
"{[C_TEST_LIST]}": "\n ".join(
"hrnAdd(%3u, %8s);" % (idx + 1, "true" if self.test in (None, idx + 1) else "false") for idx in range(module.total)

View File

@@ -11,6 +11,11 @@ import os
import yaml
from common.error import ToolError, check
from common.render import bld_enum
# Path the test modules live in, relative to the repository. Everything in it is a test module, which is what lets the linter
# report a module that was never declared here.
TEST_MODULE_PATH = "test/src/module/"
# Test types, in the order they are declared in define.yaml and accumulated
TEST_TYPE_UNIT = "unit"
@@ -362,6 +367,19 @@ def test_def_parse(path_repo):
return result
####################################################################################################################################
def test_def_file(module):
"""File a test module lives in, relative to the repository.
A C module is named in camel case, e.g. common/stack-trace is test/src/module/common/stackTraceTest.c, while a python module is
named exactly as it is declared, e.g. test/common/vm is test/src/module/test/common/vm_test.py."""
if module.lang == TEST_LANG_PYTHON:
return "%s%s_test.py" % (TEST_MODULE_PATH, module.name)
return "%s%sTest.c" % (TEST_MODULE_PATH, bld_enum(None, module.name))
####################################################################################################################################
def test_def_find(module_list, name):
"""Find a test module by name."""

View File

@@ -17,6 +17,7 @@ from command.test.define import (
TEST_LANG_PYTHON,
TEST_LIB_LIST,
TEST_TYPE_PERFORMANCE,
test_def_file,
test_def_find,
test_def_parse,
test_lib_path,
@@ -62,7 +63,7 @@ def _test_python(config, module):
It runs in a separate interpreter so the modules it declared are the only library modules it can import. Running it here would
not work since the harness has already imported most of the library and an import found in sys.modules never reaches a hook."""
path_test = os.path.join(config.repo_path, "test/src/module", module.name + "_test.py")
path_test = os.path.join(config.repo_path, test_def_file(module))
if not os.path.isfile(path_test):
raise ToolError("unable to find test module '%s'" % path_test)

View File

@@ -1,7 +1,8 @@
"""Test Code Linter.
The linter is made of the scan for content that could hide code and the check that StringId macros encode what they claim to, so
both are tested here along with the command that walks the repository and applies them."""
The linter is made of the scan for content that could hide code, the check that StringId macros encode what they claim to, and the
check that every test module is declared, so all of them are tested here along with the command that walks the repository and
applies them."""
####################################################################################################################################
import io
@@ -17,6 +18,25 @@ from command.lint.string_id import *
from common.error import *
from common.log import *
# A test definition with no test modules at all, which is what a repository has to have before the linter can scan it
_DEFINE_NONE = b"unit: []\nintegration: []\nperformance: []\ntool: []\n"
# A test definition with a module in each language, since the file a module lives in is named differently for each
_DEFINE_MODULE = b"""
unit:
- name: common/stack-trace
total: 1
integration: []
performance: []
tool:
- name: test/common/vm
coverage:
- test/common/vm
"""
####################################################################################################################################
def _capture(function):
@@ -41,6 +61,9 @@ def _capture(function):
def _lint(file_map, symlink=False):
"""Lint a repository built from the files given, returning what was reported and the error raised, if any."""
# Every repository has a test definition, so supply one with no modules unless the test is providing its own
file_map = {"test/define.yaml": _DEFINE_NONE, **file_map}
with tempfile.TemporaryDirectory() as path:
for name, content in file_map.items():
os.makedirs(os.path.dirname(os.path.join(path, name)), exist_ok=True)
@@ -252,3 +275,30 @@ def test_lint_lib_shadow():
error, output = _lint({"build/lib/common/log.py": b"", "test/lib/uncrustify.cfg": b""})
assert_is_none(error)
####################################################################################################################################
def test_lint_test_module():
"""A test module must be declared in define.yaml, since one that is not declared is never run."""
# A C module is named in camel case and a python module exactly as it is declared, so both are found where they live
file_map = {
"test/define.yaml": _DEFINE_MODULE,
"test/src/module/common/stackTraceTest.c": b"",
"test/src/module/test/common/vm_test.py": b"",
}
error, output = _lint(file_map)
assert_is_none(error)
assert_equal(output, "")
# A test module that was added but never declared, which is the case that has no other way to be reported
error, output = _lint({**file_map, "test/src/module/common/type/cTest.c": b""})
assert_equal(error, "test module 'test/src/module/common/type/cTest.c' is not defined in test/define.yaml")
# A file that is not where the test modules live is not a test module
error, output = _lint({**file_map, "test/src/harness/config.c": b""})
assert_is_none(error)

View File

@@ -182,6 +182,19 @@ def test_define_module():
assert_equal(test_def_find(module_list, "test/common/log").lang, TEST_LANG_PYTHON)
####################################################################################################################################
def test_define_file():
"""The file a module lives in is named for the module, in the way its language names it."""
module_list = _def_parse(DEFINE)
# A C module is named in camel case, which is how the C source is named
assert_equal(test_def_file(test_def_find(module_list, "common/stack-trace")), "test/src/module/common/stackTraceTest.c")
# A python module is named exactly as it is declared, since that is how it is imported
assert_equal(test_def_file(test_def_find(module_list, "test/common/log")), "test/src/module/test/common/log_test.py")
####################################################################################################################################
def test_define_option():
"""An option is inherited from the path it was declared for, innermost first, and a module may override it."""