diff --git a/.github/workflows/lint-test.yaml b/.github/workflows/lint-test.yaml index 7178c1ce7fbbd..2b9747fa3b71d 100644 --- a/.github/workflows/lint-test.yaml +++ b/.github/workflows/lint-test.yaml @@ -142,8 +142,8 @@ jobs: - uses: ./.github/actions/rust-setup - run: cargo test --locked --features check-vm-features -p aptos-node - python-unit-test: - uses: ./.github/workflows/python-unit-test.yaml + python-lint-test: + uses: ./.github/workflows/python-lint-test.yaml helm-lint: uses: ./.github/workflows/helm-lint.yaml diff --git a/.github/workflows/python-unit-test.yaml b/.github/workflows/python-lint-test.yaml similarity index 72% rename from .github/workflows/python-unit-test.yaml rename to .github/workflows/python-lint-test.yaml index 063c0a61a53f4..d750beb672174 100644 --- a/.github/workflows/python-unit-test.yaml +++ b/.github/workflows/python-lint-test.yaml @@ -32,6 +32,18 @@ jobs: python testsuite/determinator.py changed-files --github-output-key SHOULD_RUN --pattern 'testsuite/.*py' ${{steps.changed-files.outputs.all_changed_files }} id: should-run-tests + - name: Install Python Dev Dependencies + if: steps.should-run-tests.outputs.SHOULD_RUN == 'true' + run: pip install pyright black + + - name: Run python static type checker + if: steps.should-run-tests.outputs.SHOULD_RUN == 'true' + run: pyright testsuite/**/*.py + + - name: Run python fmt + if: steps.should-run-tests.outputs.SHOULD_RUN == 'true' + run: black --check --diff testsuite + - name: Run python unit tests if: steps.should-run-tests.outputs.SHOULD_RUN == 'true' run: python -m unittest testsuite/*test*.py diff --git a/testsuite/determinator.py b/testsuite/determinator.py index 6d8e748018b6a..ac50ec63aaed7 100644 --- a/testsuite/determinator.py +++ b/testsuite/determinator.py @@ -14,11 +14,14 @@ class Verdict: subverdicts: Sequence[Verdict] def format(self, indent=0) -> str: - return "\n".join([ - (indent * " ") + ("PASS" if self.verdict else "FAIL") + f"ED because {self.reason}", - ] + [ - verdict.format(indent+1) for verdict in self.subverdicts - ]) + return "\n".join( + [ + (indent * " ") + + ("PASS" if self.verdict else "FAIL") + + f"ED because {self.reason}", + ] + + [verdict.format(indent + 1) for verdict in self.subverdicts] + ) TEvaluationContext = TypeVar("TEvaluationContext") @@ -97,10 +100,7 @@ def changed_files( print(verdict.format()) if github_output_key: - output = GithubOutput( - github_output_key, - "true" if verdict.verdict else "false" - ) + output = GithubOutput(github_output_key, "true" if verdict.verdict else "false") print(output.format()) else: if not verdict.verdict: @@ -108,4 +108,4 @@ def changed_files( if __name__ == "__main__": - main() \ No newline at end of file + main() diff --git a/testsuite/determinator_test.py b/testsuite/determinator_test.py index 28108347d94f4..41349143bdebc 100644 --- a/testsuite/determinator_test.py +++ b/testsuite/determinator_test.py @@ -6,17 +6,13 @@ class ChangedFilesPredicateTestCase(unittest.TestCase): def test_changed_files_passes(self) -> None: - context: ChangedFilesContext = { - "changed_files": ["asdf"] - } + context: ChangedFilesContext = {"changed_files": ["asdf"]} predicate = ChangedFilesPredicate(["a.*f"]) verdict = predicate.evaluate(context) self.assertTrue(verdict.verdict, verdict.reason) def test_changed_files_fails(self) -> None: - context: ChangedFilesContext = { - "changed_files": ["asdf"] - } + context: ChangedFilesContext = {"changed_files": ["asdf"]} predicate = ChangedFilesPredicate(["fdas"]) verdict = predicate.evaluate(context) self.assertFalse(verdict.verdict, verdict.reason) @@ -29,15 +25,15 @@ def test_determinator_from_github(self) -> None: main, [ "changed-files", - "--github-output-key", "BANANA", - "testsuite/fixtures/helm" + "--github-output-key", + "BANANA", + "testsuite/fixtures/helm", ], - catch_exceptions=False + catch_exceptions=False, ) self.assertEqual( result.output, - "FAILED because Matched files: []\n" - "::set-output name=BANANA::false\n" + "FAILED because Matched files: []\n" "::set-output name=BANANA::false\n", ) self.assertEqual(result.exit_code, 0) @@ -47,16 +43,18 @@ def test_determinator_from_github_passes(self) -> None: main, [ "changed-files", - "--pattern", ".*/.*.ts", - "--github-output-key", "BANANA", + "--pattern", + ".*/.*.ts", + "--github-output-key", + "BANANA", "testsuite/fixtures/helm/banana.ts", ], - catch_exceptions=False + catch_exceptions=False, ) self.assertEqual( result.output, "PASSED because Matched files: " "['testsuite/fixtures/helm/banana.ts']\n" - "::set-output name=BANANA::true\n" + "::set-output name=BANANA::true\n", ) self.assertEqual(result.exit_code, 0) diff --git a/testsuite/forge.py b/testsuite/forge.py index 607f9b3a4e69d..cbb468b527f75 100644 --- a/testsuite/forge.py +++ b/testsuite/forge.py @@ -18,7 +18,8 @@ from typing import ( Any, Callable, - Dict, + Iterator, + Mapping, Generator, List, Optional, @@ -51,10 +52,10 @@ def succeeded(self) -> bool: return self.exit_code == 0 -def get_prompt_answer(prompt: str, answer: Optional[str]=None) -> bool: +def get_prompt_answer(prompt: str, answer: Optional[str] = None) -> bool: """Get a yes/no answer from the user, or use the default answer if provided.""" if not answer and not os.getenv("CI"): - answer = input(f"{prompt} (y/n) ").strip().lower() + answer = input(f"{prompt} (y/n) ").strip().lower() return answer in ("y", "yes", "yeet", "yessir", "si", "true") @@ -351,9 +352,9 @@ def milliseconds(timestamp: datetime) -> int: def apply_humio_time_filter( - urlparts: Dict[str, Union[str, bool, int]], + urlparts: Mapping[str, Union[str, bool, int]], time_filter: Union[bool, Tuple[datetime, datetime]], -) -> Dict: +) -> Mapping: if time_filter is True: urlparts = { **urlparts, @@ -1609,23 +1610,23 @@ class TestConfig(TypedDict): class TestSuite(TypedDict): name: str - all_tests: Dict[str, TestConfig] - enabled_tests: Dict[str, TestConfig] + all_tests: Mapping[str, TestConfig] + enabled_tests: Mapping[str, TestConfig] # All changes to this struct must be backwards compatible # i.e. its ok to add a new field, but not to remove one - - class ForgeConfigValue(TypedDict): enabled_clusters: List[str] all_clusters: List[str] - test_suites: Dict[str, TestSuite] - default_helm_values: Dict + test_suites: Mapping[str, TestSuite] + default_helm_values: Mapping def default_forge_config() -> ForgeConfigValue: - return { + # Return a default config with not all the fields, as they are not mandatory + # This ensures we check for backwards compatibility + return { # type: ignore "enabled_clusters": [], "all_clusters": [], } @@ -1670,7 +1671,12 @@ def ensure_forge_config(value: Any) -> ForgeConfigValue: raise Exception("Type had errors:\n" + "\n".join(errors)) return value -def get_forge_config_diff(old_config: dict, new_config: dict, full_diff: Optional[bool]=False) -> list: + +def get_forge_config_diff( + old_config: ForgeConfigValue, + new_config: ForgeConfigValue, + full_diff: Optional[bool] = False, +) -> Iterator[str]: """Returns a list of diffs between the old and new config""" config_string = json.dumps(new_config, indent=2) old_config_string = json.dumps(old_config, indent=2) @@ -1682,6 +1688,7 @@ def get_forge_config_diff(old_config: dict, new_config: dict, full_diff: Optiona else: return difflib.unified_diff(old_lines, new_lines) + class ForgeConfigBackend: def create(self) -> None: raise NotImplementedError() @@ -1887,7 +1894,7 @@ def config_edit(ctx: click.Context) -> None: shell = LocalShell(True) filesystem = LocalFilesystem() processes = SystemProcesses() - context = SystemContext(shell, filesystem, processes, time) + context = SystemContext(shell, filesystem, processes, SystemTime()) config = ForgeConfig(S3ForgeConfigBackend(context, DEFAULT_CONFIG)) config.init() @@ -1999,7 +2006,7 @@ def cluster_config_delete( shell = LocalShell() filesystem = LocalFilesystem() processes = SystemProcesses() - context = SystemContext(shell, filesystem, processes, time) + context = SystemContext(shell, filesystem, processes, SystemTime()) config = ForgeConfig(S3ForgeConfigBackend(context, DEFAULT_CONFIG)) config.init() @@ -2176,7 +2183,7 @@ def test_config_add( raise Exception(f"Test {test_name} already exists") if test_name: - test_suite["all_tests"][test_name]: TestConfig = { + test_suite["all_tests"][test_name] = { "name": test_name, } @@ -2321,6 +2328,7 @@ def test_config_enable( else: print("Config not updated") + @test_config.command("disable") @click.argument("suite_name") @click.argument("test_name") @@ -2361,5 +2369,6 @@ def test_config_disable( else: print("Config not updated") + if __name__ == "__main__": main() diff --git a/testsuite/forge_test.py b/testsuite/forge_test.py index 92b0b50181341..7f3ec7f9dd00f 100644 --- a/testsuite/forge_test.py +++ b/testsuite/forge_test.py @@ -56,7 +56,7 @@ validate_forge_config, ) -from click.testing import CliRunner +from click.testing import CliRunner, Result from forge_wrapper_core.filesystem import Filesystem from forge_wrapper_core.git import Git from forge_wrapper_core.process import Process, Processes @@ -65,7 +65,7 @@ from forge_wrapper_core.time import Time # Show the entire diff when unittest fails assertion -unittest.util._MAX_LENGTH = 2000 +unittest.util._MAX_LENGTH = 2000 # type: ignore class HasAssertMultiLineEqual(Protocol): @@ -1071,7 +1071,7 @@ def testHelmGetConfig(self) -> None: stack.enter_context( patch.object(forge, "LocalFilesystem", lambda: filesystem) ) - result_helm_config_not_present = runner.invoke( + result_helm_config_not_present: Result = runner.invoke( main, ["config", "helm", "get", "aptos-node"], catch_exceptions=True, @@ -1093,24 +1093,28 @@ def testHelmGetConfig(self) -> None: # assert that we error with a message when the config is not present self.assertEqual(result_helm_config_not_present.exit_code, 1) + self.assertIsNotNone(result_helm_config_not_present.exception) self.assertEqual( - result_helm_config_not_present.exception.args, + result_helm_config_not_present.exception.args, # type: ignore Exception("Missing key default_helm_values in Forge config").args, ) # assert that we error with a message when the config is missing partial information self.assertEqual(result_helm_config_present_missing.exit_code, 1) + self.assertIsNotNone(result_helm_config_present_missing.exception) self.assertEqual( - result_helm_config_present_missing.exception.args, + result_helm_config_present_missing.exception.args, # type: ignore Exception("No helm values found for chart aptos-genesis").args, ) # we successfully get the config self.assertEqual(result_helm_config_present_complete.exit_code, 0) + self.assertIsNotNone(helm_after_complete.get("default_helm_values")) + self.assertIsNotNone(helm_after_complete.get("default_helm_values").get("aptos-node")) # type: ignore # the output config is printed with an extra newline self.assertEqual( result_helm_config_present_complete.stdout_bytes, - f'{json.dumps(helm_after_complete.get("default_helm_values").get("aptos-node"), indent=2)}\n'.encode(), + f'{json.dumps(helm_after_complete.get("default_helm_values").get("aptos-node"), indent=2)}\n'.encode(), # type: ignore ) def testHelmSetConfig(self) -> None: @@ -1291,7 +1295,7 @@ def testHelmSetConfigPreview(self) -> None: filesystem.assert_writes(self) self.assertEqual(ret.exception, None) self.assertEqual(ret.exit_code, 0) - assert(ret.stdout_bytes.decode("utf-8").strip()) + assert ret.stdout_bytes.decode("utf-8").strip() self.assertEqual( ret.stdout_bytes.decode("utf-8").strip(), config_fixture_preview.read_bytes().decode("utf-8").strip(), diff --git a/testsuite/lint.py b/testsuite/lint.py index 2c73f8cf9382e..f3a51509575d3 100644 --- a/testsuite/lint.py +++ b/testsuite/lint.py @@ -7,7 +7,6 @@ import click - @click.group() def main() -> None: pass @@ -23,16 +22,22 @@ def helm(paths: Tuple[str]) -> None: result = shell.run(["helm", "lint", path]) for line in result.output.decode().splitlines(): if line.startswith("[ERROR]"): - match = re.match(r".ERROR. (?P
[^:]+?): (?P.*) at [(](?P.*):(?P\d+)[)]: (?P.*)", line) + match = re.match( + r".ERROR. (?P
[^:]+?): (?P.*) at [(](?P.*):(?P\d+)[)]: (?P.*)", + line, + ) if match: fullpath = Path(path).parent / match.group("filename") - print("::error file={fullpath},line={line},col=1::{message}".format(fullpath=fullpath, **match.groupdict())) + print( + "::error file={fullpath},line={line},col=1::{message}".format( + fullpath=fullpath, **match.groupdict() + ) + ) error = True if error: raise SystemExit(1) - if __name__ == "__main__": - main() \ No newline at end of file + main() diff --git a/testsuite/lint_test.py b/testsuite/lint_test.py index 94c02d44a8403..3e5e6c834d52c 100644 --- a/testsuite/lint_test.py +++ b/testsuite/lint_test.py @@ -15,16 +15,22 @@ def testHelm(self) -> None: b"[ERROR] templates/: parse error at (testnet-addons/templates/load" b"test.yaml:75): function alkajsdfl not defined" ) - shell = SpyShell(OrderedDict([ - ("helm lint testsuite/fixtures/helm", RunResult(0, error)), - ])) + shell = SpyShell( + OrderedDict( + [ + ("helm lint testsuite/fixtures/helm", RunResult(0, error)), + ] + ) + ) with patch.object(lint, "LocalShell", lambda *_: shell): runner = CliRunner() - result = runner.invoke(main, ["helm", "testsuite/fixtures/helm"], catch_exceptions=False) + result = runner.invoke( + main, ["helm", "testsuite/fixtures/helm"], catch_exceptions=False + ) shell.assert_commands(self) expected_error = ( "::error file=testsuite/fixtures/testnet-addons/templates/loadtest." "yaml,line=75,col=1::function alkajsdfl not defined\n" ) - self.assertEqual(result.output, expected_error) \ No newline at end of file + self.assertEqual(result.output, expected_error)