Skip to content

Commit

Permalink
Mark one test from mostly-non-JS/JVM backends as platform-specific be…
Browse files Browse the repository at this point in the history
…haviour, test on macOS (#21610)

This goes through many backends and marks one test as
`platform_specific_behavior` to ensure the backends have some testing on
platforms other than x86-64 Linux (macOS, and arm64 Linux).

The particular value in doing this is validating the set-up code, e.g.
got the right configuration for installing/downloading the tool, rather
than validating the details of the behaviour. I think we generally
assume most tools behave similar across platforms, once they're up and
running.

This PR focuses on non-JVM and non-JS backends, meaning raw binaries
(`ExternalTool`m `TemplatedExternalTool`) and Python tools installed
from PyPI (`PythonToolBase`, `PythonToolRequirementsBase`). It
particularly focuses on the backends that don't require installing any
additional software in CI, see #21620 and #21621 for additional backends
that do require installing Go and Docker respectively.

The impression I get is the JVM and JS tools are more
platform-independent, often? For instance, their lockfiles/artifacts
don't mention specific platforms, unlike Python wheels.

I found these backends via the following shell command, plus other
exploration:

```shell
for file in $(rg --files-with-matches "class.*(PythonTool.*Base|ExternalTool)" --glob '*.py' . | sed 's@/[^/]*$@@' | sort | uniq); do
  rg -q "platform_specific_behavior" $file || echo $file
done
```

I may've missed some!

This increases the time to run tests in CI on the relevant platforms, by
slight more than 2×.

| platform     | before                 | after         |
|--------------|------------------------|---------------|
| Linux x86-64 | runs all tests always  | no change     |
| Linux arm64  | ~3:30                  | ~7:30         |
| macOS x86-64 | ~5:30                  | 12:00 - 13:30 |
| macOS arm64  | not run in CI (#20993) | no change     |

Related: #21608, #20193, #20993
  • Loading branch information
huonw authored Nov 8, 2024
1 parent 69043a7 commit d28e5c3
Show file tree
Hide file tree
Showing 39 changed files with 151 additions and 17 deletions.
9 changes: 8 additions & 1 deletion src/python/pants/backend/build_files/fmt/buildifier/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,11 @@

python_sources()

python_tests(name="tests")
python_tests(
name="tests",
overrides={
"rules_integration_test.py": {
"tags": ["platform_specific_behavior"],
}
},
)
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ def test_passing(rule_runner: RuleRunner) -> None:
assert fmt_result.did_change is False


@pytest.mark.platform_specific_behavior
def test_failing(rule_runner: RuleRunner) -> None:
rule_runner.write_files({"BUILD": BAD_FILE})
fmt_result = run_buildifier(rule_runner)
Expand Down
9 changes: 8 additions & 1 deletion src/python/pants/backend/codegen/protobuf/lint/buf/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,11 @@
# Licensed under the Apache License, Version 2.0 (see LICENSE).

python_sources()
python_tests(name="tests")
python_tests(
name="tests",
overrides={
"lint_rules_integration_test.py": {
"tags": ["platform_specific_behavior"],
}
},
)
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ def test_passing(rule_runner: RuleRunner) -> None:
assert_success(rule_runner, tgt)


@pytest.mark.platform_specific_behavior
def test_failing(rule_runner: RuleRunner) -> None:
rule_runner.write_files(
{"foo/v1/f.proto": BAD_FILE, "foo/v1/BUILD": "protobuf_sources(name='t')"}
Expand Down
5 changes: 5 additions & 0 deletions src/python/pants/backend/cue/goals/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,9 @@ python_sources()

python_tests(
name="tests",
overrides={
"fix_test.py": {
"tags": ["platform_specific_behavior"],
}
},
)
1 change: 1 addition & 0 deletions src/python/pants/backend/cue/goals/fix_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ def test_simple_cue_fmt(rule_runner: RuleRunner) -> None:
)


@pytest.mark.platform_specific_behavior
def test_simple_cue_fmt_issue(rule_runner: RuleRunner) -> None:
rule_runner.write_files(
{
Expand Down
10 changes: 9 additions & 1 deletion src/python/pants/backend/helm/check/kubeconform/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,12 @@

python_sources()

python_tests(name="tests", timeout=120)
python_tests(
name="tests",
timeout=120,
overrides={
"deployment_test.py": {
"tags": ["platform_specific_behavior"],
}
},
)
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ def test_valid_deployment(rule_runner: RuleRunner) -> None:
)


@pytest.mark.platform_specific_behavior
def test_invalid(rule_runner: RuleRunner) -> None:
rule_runner.write_files(
{
Expand Down
12 changes: 11 additions & 1 deletion src/python/pants/backend/helm/goals/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,14 @@

python_sources()

python_tests(name="tests", overrides={"deploy_test.py": {"timeout": 180}})
python_tests(
name="tests",
overrides={
"deploy_test.py": {
"timeout": 180,
},
"package_test.py": {
"tags": ["platform_specific_behavior"],
},
},
)
1 change: 1 addition & 0 deletions src/python/pants/backend/helm/goals/package_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ def _assert_build_package(rule_runner: RuleRunner, *, chart_name: str, chart_ver
assert result.artifacts[0].info


@pytest.mark.platform_specific_behavior
def test_helm_package(rule_runner: RuleRunner) -> None:
chart_name = "foo"
chart_version = "0.1.0"
Expand Down
10 changes: 7 additions & 3 deletions src/python/pants/backend/javascript/goals/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,12 @@ python_tests(
name="tests",
# The package.json files are inlined in ./test_integration_tests.py
overrides={
"test_integration_test.py": dict(
dependencies=["./jest_resources", "./mocha_resources"], timeout=240
),
"test_integration_test.py": {
"dependencies": ["./jest_resources", "./mocha_resources"],
"timeout": 240,
},
"export_test.py": {
"tags": ["platform_specific_behavior"],
},
},
)
1 change: 1 addition & 0 deletions src/python/pants/backend/javascript/goals/export_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ def get_snapshot(rule_runner: RuleRunner, digest: Digest) -> Snapshot:
return rule_runner.request(Snapshot, [digest])


@pytest.mark.platform_specific_behavior
def test_export_node_modules(rule_runner: RuleRunner) -> None:
rule_runner.set_options(["--export-resolve=nodejs-default"], env_inherit={"PATH"})
rule_runner.write_files(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ def given_package_json(
)


@pytest.mark.platform_specific_behavior
@pytest.mark.parametrize(
"test_script, package_json_target",
[
Expand Down
5 changes: 5 additions & 0 deletions src/python/pants/backend/makeself/goals/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,9 @@ python_sources()

python_tests(
name="tests",
overrides={
"package_run_integration_test.py": {
"tags": ["platform_specific_behavior"],
}
},
)
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ def rule_runner() -> RuleRunner:
return rule_runner


@pytest.mark.platform_specific_behavior
def test_simple_archive(rule_runner: RuleRunner) -> None:
binary_name = "archive"

Expand Down
9 changes: 8 additions & 1 deletion src/python/pants/backend/nfpm/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,11 @@

python_sources()

python_tests(name="tests")
python_tests(
name="tests",
overrides={
"rules_integration_test.py": {
"tags": ["platform_specific_behavior"],
}
},
)
1 change: 1 addition & 0 deletions src/python/pants/backend/nfpm/rules_integration_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ def test_generate_package_without_contents(
_assert_one_built_artifact(_PKG_NAME, built_package, field_set_type)


@pytest.mark.platform_specific_behavior
@pytest.mark.parametrize("field_set_type,extra_metadata,valid_target", _TEST_CASES)
def test_generate_package_with_contents(
rule_runner: RuleRunner,
Expand Down
7 changes: 6 additions & 1 deletion src/python/pants/backend/project_info/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ python_tests(
name="tests",
timeout=180,
overrides={
"list_targets_test.py": {"dependencies": ["//BUILD_ROOT:files"]},
"list_targets_test.py": {
"dependencies": ["//BUILD_ROOT:files"],
},
"count_loc_test.py": {
"tags": ["platform_specific_behavior"],
},
},
)
1 change: 1 addition & 0 deletions src/python/pants/backend/project_info/count_loc_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ def assert_counts(
assert code == int(fields[5])


@pytest.mark.platform_specific_behavior
def test_count_loc(rule_runner: RuleRunner) -> None:
py_dir = "src/py/foo"
elixir_dir = "src/elixir/foo"
Expand Down
1 change: 1 addition & 0 deletions src/python/pants/backend/python/providers/pyenv/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ python_tests(
overrides={
"rules_integration_test.py": {
"timeout": 600,
"tags": ["platform_specific_behavior"],
}
},
)
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ def run_run_request(
return mocked_console[1].get_stdout().strip()


@pytest.mark.platform_specific_behavior
@pytest.mark.parametrize(
"interpreter_constraints, expected_version_substring",
[("2.7.*", "2.7"), ("3.9.*", "3.9"), ("3.10.4", "3.10.4")],
Expand Down
5 changes: 4 additions & 1 deletion src/python/pants/backend/python/typecheck/pytype/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ python_sources(
python_tests(
name="tests",
overrides={
"rules_integration_test.py": {"timeout": 240},
"rules_integration_test.py": {
"timeout": 240,
"tags": ["platform_specific_behavior"],
}
},
)
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ def test_passing(rule_runner: PythonRuleRunner) -> None:
assert result[0].report == EMPTY_DIGEST


@pytest.mark.platform_specific_behavior
def test_failing(rule_runner: PythonRuleRunner) -> None:
rule_runner.write_files({f"{PACKAGE}/f.py": BAD_FILE, f"{PACKAGE}/BUILD": "python_sources()"})
tgt = rule_runner.get_target(Address(PACKAGE, relative_file_path="f.py"))
Expand Down
9 changes: 8 additions & 1 deletion src/python/pants/backend/shell/goals/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,11 @@
# Licensed under the Apache License, Version 2.0 (see LICENSE).

python_sources()
python_tests(name="tests")
python_tests(
name="tests",
overrides={
"test_test.py": {
"tags": ["platform_specific_behavior"],
}
},
)
1 change: 1 addition & 0 deletions src/python/pants/backend/shell/goals/test_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ def rule_runner() -> RuleRunner:
return rule_runner


@pytest.mark.platform_specific_behavior
def test_shell_command_as_test(rule_runner: RuleRunner) -> None:
rule_runner.write_files(
{
Expand Down
9 changes: 8 additions & 1 deletion src/python/pants/backend/shell/lint/shellcheck/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,11 @@
# Licensed under the Apache License, Version 2.0 (see LICENSE).

python_sources()
python_tests(name="tests")
python_tests(
name="tests",
overrides={
"rules_integration_test.py": {
"tags": ["platform_specific_behavior"],
}
},
)
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ def test_passing(rule_runner: RuleRunner) -> None:
assert_success(rule_runner, tgt)


@pytest.mark.platform_specific_behavior
def test_failing(rule_runner: RuleRunner) -> None:
rule_runner.write_files({"f.sh": BAD_FILE, "BUILD": "shell_sources(name='t')"})
tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.sh"))
Expand Down
9 changes: 8 additions & 1 deletion src/python/pants/backend/shell/lint/shfmt/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,11 @@
# Licensed under the Apache License, Version 2.0 (see LICENSE).

python_sources()
python_tests(name="tests")
python_tests(
name="tests",
overrides={
"rules_integration_test.py": {
"tags": ["platform_specific_behavior"],
}
},
)
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ def test_passing(rule_runner: RuleRunner) -> None:
assert fmt_result.did_change is False


@pytest.mark.platform_specific_behavior
def test_failing(rule_runner: RuleRunner) -> None:
rule_runner.write_files({"f.sh": BAD_FILE, "BUILD": "shell_sources(name='t')"})
tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.sh"))
Expand Down
5 changes: 5 additions & 0 deletions src/python/pants/backend/terraform/lint/tfsec/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,9 @@ python_sources()

python_tests(
name="tests",
overrides={
"tfsec_integration_test.py": {
"tags": ["platform_specific_behavior"],
}
},
)
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
# Licensed under the Apache License, Version 2.0 (see LICENSE).
from textwrap import dedent

import pytest

from pants.backend.terraform import tool
from pants.backend.terraform.lint.tffmt.tffmt import PartitionMetadata
from pants.backend.terraform.lint.tfsec.rules import rules as tfsec_rules
Expand Down Expand Up @@ -83,6 +85,7 @@ def set_up_rule_runner(tfsec_args: list[str]) -> RuleRunner:
return rule_runner


@pytest.mark.platform_specific_behavior
def test_run_tfsec():
rule_runner = set_up_rule_runner([])

Expand Down
5 changes: 4 additions & 1 deletion src/python/pants/backend/tools/semgrep/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ python_sources(dependencies=[":lockfile"])
python_tests(
name="tests",
overrides={
"rules_integration_test.py": dict(timeout=240),
"rules_integration_test.py": {
"timeout": 240,
"tags": ["platform_specific_behavior"],
}
},
)
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ def test_passing(rule_runner: RuleRunner, major_minor_interpreter: str) -> None:
)


@pytest.mark.platform_specific_behavior
@pytest.mark.parametrize(
"files,config_name",
[
Expand Down
9 changes: 8 additions & 1 deletion src/python/pants/backend/tools/taplo/BUILD
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
# Copyright 2023 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).
python_sources()
python_tests(name="tests")
python_tests(
name="tests",
overrides={
"rules_integration_test.py": {
"tags": ["platform_specific_behavior"],
}
},
)
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ def test_no_changes_needed(rule_runner: RuleRunner) -> None:
assert fmt_result.did_change is False


@pytest.mark.platform_specific_behavior
def test_changes_needed(rule_runner: RuleRunner) -> None:
rule_runner.write_files({"f.toml": BAD_FILE, "sub/g.toml": BAD_FILE})
fmt_result = run_taplo(rule_runner)
Expand Down
9 changes: 8 additions & 1 deletion src/python/pants/backend/tools/trufflehog/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,11 @@
# Licensed under the Apache License, Version 2.0 (see LICENSE).
python_sources()

python_tests(name="tests")
python_tests(
name="tests",
overrides={
"rules_integration_test.py": {
"tags": ["platform_specific_behavior"],
}
},
)
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ def test_detectors_loaded(rule_runner: RuleRunner) -> None:
assert TOTAL_DETECTORS + 1 == extract_total_detector_count(fmt_result.stderr)


@pytest.mark.platform_specific_behavior
def test_secret_detected(rule_runner: RuleRunner) -> None:
# Write the configuration file
rule_runner.write_files(
Expand Down
9 changes: 8 additions & 1 deletion src/python/pants/backend/tools/yamllint/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,11 @@ resource(name="lockfile", source="yamllint.lock")

python_sources(dependencies=[":lockfile"])

python_tests(name="tests")
python_tests(
name="tests",
overrides={
"rules_integration_test.py": {
"tags": ["platform_specific_behavior"],
}
},
)
Loading

0 comments on commit d28e5c3

Please sign in to comment.