Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions .github/workflows/scripts/pytest-selected.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import json
import math
import os
import shlex
import sys
from dataclasses import dataclass, field
from pathlib import Path
Expand Down Expand Up @@ -161,8 +162,31 @@ def file_weights(files: list[str]) -> dict[str, float]:
return weights


def per_file_plan_applies(args: list[str], files: list[str]) -> bool:
"""Whether planning by file gives each shard the tests pytest would run.

`-m`/`-k` filter after the plan, so most shards could end up with no
selected tests (e.g. `-m snuba_ci` in Snuba's CI). The sentry plugin
matches the planned paths relative to the rootdir, which
pytest_shard_plugin checks is the working directory.
"""
every_arg = [*args, *shlex.split(os.environ.get("PYTEST_ADDOPTS", ""))]
if any(arg.startswith(("-m", "-k")) and not arg.startswith("--") for arg in every_arg):
return False
return not any(os.path.isabs(f) or f.startswith(os.pardir + os.sep) for f in files)


def run_full_shard(args: list[str], total: int, group: int) -> int:
files = list_test_files(args)
if not per_file_plan_applies(args, files):
# As before per-file sharding: every shard collects everything and the
# sentry plugin keeps its hash share of the items.
print(
"pytest-selected: -m/-k or paths outside the working directory, so "
"every shard collects everything and keeps its hash share",
file=sys.stderr,
)
return int(pytest.main(args))
plan = plan_full_shards(file_weights(files), total) if files else None
if plan is None:
return int(pytest.main(args))
Expand Down
11 changes: 11 additions & 0 deletions .github/workflows/scripts/pytest_shard_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import json
import os
from hashlib import sha256
from pathlib import Path

import pytest

Expand All @@ -19,6 +20,16 @@ def item_part(nodeid: str, parts: int) -> int:
return int(sha256(nodeid.encode()).hexdigest(), 16) % parts


def pytest_configure(config: pytest.Config) -> None:
# pytest-selected.py writes paths relative to the working directory, and
# the sentry plugin matches them relative to the rootdir.
if os.environ.get("PYTEST_SHARD_PARTS") and config.rootpath != Path.cwd().resolve():
raise pytest.UsageError(
f"pytest_shard_plugin: rootdir {config.rootpath} is not the working "
f"directory {Path.cwd()}; run pytest-selected.py from the rootdir"
)


def pytest_collection_modifyitems(config: pytest.Config, items: list[pytest.Item]) -> None:
parts_file = os.environ.get("PYTEST_SHARD_PARTS")
if not parts_file:
Expand Down
18 changes: 17 additions & 1 deletion .github/workflows/scripts/report-backend-test-failures.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,28 @@ export function parseFailures(files, core) {
.filter(t => t.outcome === 'failed')
.map(t => ({
nodeid: t.nodeid ?? 'unknown',
longrepr: (t.call ?? t.setup ?? t.teardown ?? {}).longrepr ?? '',
longrepr:
failedSubtests(t) || ((t.call ?? t.setup ?? t.teardown ?? {}).longrepr ?? ''),
artifactDir,
}));
});
}

// The parent's own longrepr only says "contains N failed subtests". Render
// each failed subtest the way pytest's SUBFAILED line does.
function failedSubtests(test) {
return (test.subtests ?? [])
.filter(s => s.outcome === 'failed')
.map(({context, longrepr}) => {
const kwargs = Object.entries(context?.kwargs ?? {}).map(([k, v]) => `${k}=${v}`);
const msg =
context?.msg === null || context?.msg === undefined ? [] : [`[${context.msg}]`];
const description = [...msg, ...(kwargs.length ? [`(${kwargs.join(', ')})`] : [])];
return `SUBFAILED ${description.join(' ') || '(<subtest>)'}\n${longrepr ?? ''}`;
})
.join('\n\n');
}

// Maps artifact directory names to GitHub Actions job log URLs.
const JOB_MATCHERS = [
{
Expand Down
28 changes: 28 additions & 0 deletions .github/workflows/scripts/report-backend-test-failures.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,34 @@ describe('parseFailures', () => {

assert.equal(parseFailures([file1, file2], mockCore()).length, 2);
});

it('uses failed subtests instead of the parent summary', () => {
const file = createArtifactFile(
'pytest-results-backend-111-0',
'pytest.json',
makePytestJson([
{
nodeid: 'tests/a.py::test_a',
outcome: 'failed',
call: {outcome: 'failed', longrepr: 'contains 1 failed subtest'},
subtests: [
{outcome: 'passed', context: {msg: null, kwargs: {n: '1'}}},
{
outcome: 'failed',
longrepr: 'AssertionError: boom',
context: {msg: 'm', kwargs: {f: '<function f>', n: '2'}},
},
],
},
])
);

const [failure] = parseFailures([file], mockCore());
assert.equal(
failure.longrepr,
'SUBFAILED [m] (f=<function f>, n=2)\nAssertionError: boom'
);
});
});

describe('extractNodeids', () => {
Expand Down
8 changes: 6 additions & 2 deletions .github/workflows/scripts/test_backend_test_durations.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@

import importlib
import json
from pathlib import Path

# Module has a hyphen in its name, so use importlib.
_mod = importlib.import_module("backend-test-durations")


def test_extract_sums_stages_per_file(tmp_path):
def test_extract_sums_stages_per_file(tmp_path: Path) -> None:
report = tmp_path / "pytest.json"
report.write_text(
json.dumps(
Expand All @@ -18,6 +19,9 @@ def test_extract_sums_stages_per_file(tmp_path):
"setup": {"duration": 1.0},
"call": {"duration": 2.0},
"teardown": {"duration": 0.5},
# Subtests ran inside the call; reruns don't count.
"subtests": [{"duration": 1.5}],
"reruns": [{"call": {"duration": 9.0}}],
},
{"nodeid": "tests/a.py::test_2[x]", "setup": {"duration": 0.5}},
{"nodeid": "tests/b.py::test_3", "call": {"duration": 4.0}},
Expand All @@ -28,7 +32,7 @@ def test_extract_sums_stages_per_file(tmp_path):
assert _mod.extract(report) == {"files": {"tests/a.py": 4.0, "tests/b.py": 4.0}, "tests": 3}


def test_merge_adds_up_shards():
def test_merge_adds_up_shards() -> None:
shards = [
{"files": {"tests/a.py": 1.04, "tests/b.py": 2.0}, "tests": 2},
{"files": {"tests/a.py": 1.0}, "tests": 2},
Expand Down
Loading
Loading