Run go tests directly in PRESUBMIT.py Following the addition of the golang CIPD package in crrev.com/c/7800167, even the presubmit bot can run go functionality via third_party/golang/bin/go. So let's move all the go tests to that bot. See also crrev.com/c/7806673 in catapult and crrev.com/c/7806632 in recipes. CheckPrebuiltBinaryUpdated() is updated to run only on submission (by adding OnCommit to the name), similar to what was initially implemented in crrev.com/c/7802746. One interesting note is that, although this doesn't run on `git cl upload`, it runs on explicit `git cl presubmit` (TIL). This CL also removes CheckBuildpWpr() and CheckBuildHttpArchive(), which are redundant with CheckGoTests(). Bug: 495366518 Change-Id: Ie8e441a76f9722d13aab4a006f804460fc2d681a Reviewed-on: https://chromium-review.googlesource.com/c/webpagereplay/+/7807226 Auto-Submit: Victor Vianna <victorvianna@google.com> Commit-Queue: Mikhail Khokhlov <khokhlov@google.com> Reviewed-by: Mikhail Khokhlov <khokhlov@google.com>
diff --git a/PRESUBMIT.py b/PRESUBMIT.py index d2a89ae..184f86b 100644 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py
@@ -7,104 +7,56 @@ for more details about the presubmit API built into depot_tools. """ -import os import pathlib -import tempfile PRESUBMIT_VERSION = '2.0.0' USE_PYTHON3 = True -# TODO(crbug.com/495366518): The presubmit bot doesn't contain `go`, so -# go-related checks are failing. We should fix by moving these tests to a -# separate builder or installing go in the builder. For now, this check tries -# to ensure the tests are run locally by the author and skipped on bots. -def _IsRunningOnBot(): - return 'SWARMING_TASK_ID' in os.environ - -def CheckBuildpWpr(input_api, output_api): - if _IsRunningOnBot(): - return [] - - # Note: CheckGoTests() doesn't build the main function, that's why this - # separate check exists. - cmd_name = 'Test wpr builds' - with tempfile.TemporaryDirectory() as tmpdir: - out_path = str(pathlib.PurePath(tmpdir) / "wpr") - test_cmd = input_api.Command( - name=cmd_name, - cmd=['go', 'build', '-o', out_path, './src/wpr.go'], - kwargs={'cwd': input_api.PresubmitLocalPath()}, - message=output_api.PresubmitError) - return input_api.RunTests([test_cmd]) - - -def CheckBuildHttpArchive(input_api, output_api): - if _IsRunningOnBot(): - return [] - - # Note: CheckGoTests() doesn't build the main function, that's why this - # separate check exists. - cmd_name = 'Test httparchive builds' - with tempfile.TemporaryDirectory() as tmpdir: - out_path = str(pathlib.Path(tmpdir) / "httparchive") - test_cmd = input_api.Command( - name=cmd_name, - cmd=['go', 'build', '-o', out_path, './src/httparchive.go'], - kwargs={'cwd': input_api.PresubmitLocalPath()}, - message=output_api.PresubmitError) - return input_api.RunTests([test_cmd]) - - def CheckGoTests(input_api, output_api): - if _IsRunningOnBot(): - return [] + # All commands below are run from the src/ directory, this path is relative + # to that. + go_path = '../third_party/golang/bin/go' + return input_api.RunTests([ + input_api.Command( + name='webpagereplay package tests', + cmd=[go_path, 'test', './webpagereplay'], + kwargs={ + 'cwd': + str(pathlib.Path(input_api.PresubmitLocalPath()) / 'src') + }, + message=output_api.PresubmitError), + input_api.Command( + name='wpr.go tests', + cmd=[go_path, 'test', 'wpr.go', 'wpr_test.go'], + kwargs={ + 'cwd': + str(pathlib.Path(input_api.PresubmitLocalPath()) / 'src') + }, + message=output_api.PresubmitError), + input_api.Command( + name='httparchive tests', + cmd=[go_path, 'test', 'httparchive.go', 'httparchive_test.go'], + kwargs={ + 'cwd': + str(pathlib.Path(input_api.PresubmitLocalPath()) / 'src') + }, + message=output_api.PresubmitError) + ]) - cmd_name = 'WebPageReplay go tests' + +# This one should only run on commit to prevent authors from having to update +# the binaries on every new patchset during review +def CheckPrebuiltBinaryUpdatedOnCommit(input_api, output_api): + cmd = ["scripts/upload_new_binaries.py", "--check-only"] if input_api.verbose: - print(f'Running {cmd_name}') - results = [] - results.extend( - input_api.RunTests([ - input_api.Command( - name=cmd_name, - cmd=['go', 'test', './webpagereplay'], - kwargs={ - 'cwd': - str(pathlib.Path(input_api.PresubmitLocalPath()) / 'src') - }, - message=output_api.PresubmitError), - input_api.Command( - name='wpr.go tests', - cmd=['go', 'test', 'wpr.go', 'wpr_test.go'], - kwargs={ - 'cwd': - str(pathlib.Path(input_api.PresubmitLocalPath()) / 'src') - }, - message=output_api.PresubmitError), - input_api.Command( - name='httparchive tests', - cmd=['go', 'test', 'httparchive.go', 'httparchive_test.go'], - kwargs={ - 'cwd': - str(pathlib.Path(input_api.PresubmitLocalPath()) / 'src') - }, - message=output_api.PresubmitError) - ])) - return results - - -def CheckPrebuiltBinaryUpdated(input_api, output_api): - files = input_api.UnixLocalPaths() - if (not any(f.endswith('binary_dependencies.json') for f in files) and any( - f.endswith('.go') and not f.endswith('_test.go') for f in files)): - return [ - output_api.PresubmitError( - 'You changed go files, but didn\'t run scripts/' - 'upload_new_binaries.py') - ] - - return [] + cmd.append("--verbose") + return input_api.RunTests([ + input_api.Command(name="check prebuilt binaries updated", + cmd=cmd, + kwargs={'cwd': input_api.PresubmitLocalPath()}, + message=output_api.PresubmitError) + ]) def CheckPanProjectChecks(input_api, output_api): @@ -127,23 +79,21 @@ def CheckGoFormat(input_api, output_api): - if _IsRunningOnBot(): - return [] - - cmd_name = 'Checking go format' - test_cmd = input_api.Command( - name=cmd_name, - cmd=['scripts/check_go_format.py'], - kwargs={'cwd': input_api.PresubmitLocalPath()}, - message=output_api.PresubmitError) - return input_api.RunTests([test_cmd]) + return input_api.RunTests([ + input_api.Command(name='Checking go format', + cmd=['scripts/check_go_format.py'], + kwargs={'cwd': input_api.PresubmitLocalPath()}, + message=output_api.PresubmitError) + ]) def CheckRuff(input_api, output_api): - cmd_name = 'Checking ruff format' - test_cmd = input_api.Command( - name=cmd_name, - cmd=['vpython3', '-m', 'ruff', 'check', '--select', 'E,F,W,B,I,UP'], - kwargs={'cwd': input_api.PresubmitLocalPath()}, - message=output_api.PresubmitError) - return input_api.RunTests([test_cmd]) + return input_api.RunTests([ + input_api.Command(name='Checking ruff format', + cmd=[ + 'vpython3', '-m', 'ruff', 'check', '--select', + 'E,F,W,B,I,UP' + ], + kwargs={'cwd': input_api.PresubmitLocalPath()}, + message=output_api.PresubmitError) + ])
diff --git a/scripts/check_go_format.py b/scripts/check_go_format.py index 887fa3c..d47fd30 100755 --- a/scripts/check_go_format.py +++ b/scripts/check_go_format.py
@@ -11,9 +11,13 @@ """ Checks for unformatted Go files using `gofmt -l`. If any are found, prints them and exits with an error. + + Why not invoke gofmt directly in PRESUBMIT.py instead of having this script? + Well, input_api.Command() relies on exit codes to signal success, but + `gofmt -l` exits with 0 even when files are unformatted. """ try: - result = subprocess.run(['gofmt', '-l', 'src'], + result = subprocess.run(['third_party/golang/bin/gofmt', '-l', 'src'], capture_output=True, text=True, check=False) @@ -27,8 +31,7 @@ print("All Go files are correctly formatted.") sys.exit(0) except FileNotFoundError: - print("Error: The 'gofmt' command was not found. Please ensure Go is " - "installed and in your system's PATH.") + print("Error: The 'gofmt' command was not found. Run `gclient sync`") sys.exit(1) except Exception as e: print(f"An unexpected error occurred: {e}")