Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: get rid of hardcoded attribute filtering in find_tasks #225

Merged
merged 1 commit into from
Dec 16, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,12 @@ def make_firefoxci_artifact_tasks(config, tasks):
for task in tasks:
# Format: { <task id>: [<artifact name>]}
tasks_to_create = defaultdict(list)
include_attrs = task.pop("include-attrs", {})
exclude_attrs = task.pop("exclude-attrs", {})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be done in a follow-up, but it might be nice to group include-attrs/exclude-attrs with individual index paths. E.g:

from-decision-indices:
    - path: <index path>
      include-attrs: [<attrs>]
      exclude-attrs: [<attrs>]

This would allow a bit more flexibility.

for decision_index_path in task.pop("decision-index-paths"):
for task_def in find_tasks(decision_index_path):
for task_def in find_tasks(
decision_index_path, include_attrs, exclude_attrs
):
# Add docker images
if "image" in task_def["payload"]:
image = task_def["payload"]["image"]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,10 @@ def schedule_tasks_at_index(config, tasks):
return

for task in tasks:
include_attrs = task.pop("include-attrs", {})
exclude_attrs = task.pop("exclude-attrs", {})
for decision_index_path in task.pop("decision-index-paths"):
for task_def in find_tasks(decision_index_path):
for task_def in find_tasks(
decision_index_path, include_attrs, exclude_attrs
):
yield make_integration_test_description(task_def, task["name"])
33 changes: 26 additions & 7 deletions taskcluster/fxci_config_taskgraph/util/integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import requests
import taskcluster
from taskgraph.util.attributes import attrmatch

from fxci_config_taskgraph.util.constants import FIREFOXCI_ROOT_URL

Expand All @@ -18,8 +19,11 @@ def get_taskcluster_client(service: str):


@cache
def find_tasks(decision_index_path: str) -> list[dict[str, Any]]:
"""Find tasks targeted by the Decision task pointed to by `decision_index_path`."""
def _fetch_task_graph(decision_index_path: str) -> list[dict[str, Any]]:
"""Fetch a decision task's `task-graph.json` given by the
`decision-index-path`. This is done separately from `find_tasks`
because the @cache decorator does not work with the `dict` parameters
in `find_tasks`."""
queue = get_taskcluster_client("queue")
index = get_taskcluster_client("index")

Expand All @@ -37,14 +41,29 @@ def find_tasks(decision_index_path: str) -> list[dict[str, Any]]:
else:
task_graph = response

return task_graph.values()


def find_tasks(
decision_index_path: str,
include_attrs: dict[str, list[str]],
exclude_attrs: dict[str, list[str]],
) -> list[dict[str, Any]]:
"""Find tasks targeted by the Decision task pointed to by `decision_index_path`
that match the the included and excluded attributes given.
"""
tasks = []
for task in task_graph.values():

for task in _fetch_task_graph(decision_index_path):
assert isinstance(task, dict)
attributes = task.get("attributes", {})
if attributes.get("unittest_variant") != "os-integration":
continue

if attributes.get("test_platform", "").startswith(("android-hw", "macosx")):
excludes = {
key: lambda attr: any([attr.startswith(v) for v in values])
for key, values in exclude_attrs.items()
}
if not attrmatch(attributes, **include_attrs) or attrmatch(
attributes, **excludes
):
continue

tasks.append(task["task"])
Expand Down
7 changes: 7 additions & 0 deletions taskcluster/kinds/firefoxci-artifact/kind.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,10 @@ tasks:
description: "Fetch artifacts from Firefox-CI for the integration tests"
decision-index-paths:
- gecko.v2.mozilla-central.latest.taskgraph.decision-os-integration
include-attrs:
unittest_variant:
- os-integration
exclude-attrs:
test_platform:
- android-hw
- macosx
7 changes: 7 additions & 0 deletions taskcluster/kinds/integration-test/kind.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,10 @@ tasks:
description: "Run Gecko integration tests"
decision-index-paths:
- gecko.v2.mozilla-central.latest.taskgraph.decision-os-integration
include-attrs:
unittest_variant:
- os-integration
exclude-attrs:
test_platform:
- android-hw
- macosx
14 changes: 11 additions & 3 deletions taskcluster/test/test_transforms_firefoxci_artifact.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

from fxci_config_taskgraph.transforms.firefoxci_artifact import transforms
from fxci_config_taskgraph.util.constants import FIREFOXCI_ROOT_URL, STAGING_ROOT_URL
from fxci_config_taskgraph.util.integration import find_tasks
from fxci_config_taskgraph.util.integration import _fetch_task_graph


@pytest.fixture
Expand Down Expand Up @@ -47,8 +47,14 @@ def run_test(monkeypatch, run_transform, responses):
json={"taskId": decision_task_id},
)

def inner(task: dict[str, Any]) -> dict[str, Any] | None:
find_tasks.cache_clear()
def inner(
task: dict[str, Any],
include_attrs: dict[str, list[str]] = {"unittest_variant": ["os-integration"]},
exclude_attrs: dict[str, list[str]] = {
"test_platform": ["android-hw", "macosx"]
},
) -> dict[str, Any] | None:
_fetch_task_graph.cache_clear()

task = merge(deepcopy(base_task), task)
task_graph = {task_label: task}
Expand All @@ -62,6 +68,8 @@ def inner(task: dict[str, Any]) -> dict[str, Any] | None:
transform_task = {
"name": "gecko",
"decision-index-paths": [index],
"include-attrs": include_attrs,
"exclude-attrs": exclude_attrs,
"worker": {
"env": {
"MOZ_ARTIFACT_DIR": "/builds/worker/artifacts",
Expand Down
29 changes: 22 additions & 7 deletions taskcluster/test/test_transforms_integration_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

from fxci_config_taskgraph.transforms.integration_test import transforms
from fxci_config_taskgraph.util.constants import FIREFOXCI_ROOT_URL, STAGING_ROOT_URL
from fxci_config_taskgraph.util.integration import find_tasks
from fxci_config_taskgraph.util.integration import _fetch_task_graph


@pytest.fixture
Expand Down Expand Up @@ -51,8 +51,15 @@ def run_test(monkeypatch, run_transform, responses):
# this comes from the keys in a `kind`. `task_label` is not really
# an ideal default, but it's the best option we have here, and this value
# is irrelevant to many tests anyways.
def inner(task: dict[str, Any], name: str = task_label) -> dict[str, Any] | None:
find_tasks.cache_clear()
def inner(
task: dict[str, Any],
include_attrs: dict[str, list[str]] = {"unittest_variant": ["os-integration"]},
exclude_attrs: dict[str, list[str]] = {
"test_platform": ["android-hw", "macosx"]
},
name: str = task_label,
) -> dict[str, Any] | None:
_fetch_task_graph.cache_clear()

task = merge(deepcopy(base_task), task)
task_graph = {task_label: task}
Expand All @@ -64,7 +71,13 @@ def inner(task: dict[str, Any], name: str = task_label) -> dict[str, Any] | None
)

result = run_transform(
transforms, {"decision-index-paths": [index], "name": name}
transforms,
{
"decision-index-paths": [index],
"include-attrs": include_attrs,
"exclude-attrs": exclude_attrs,
"name": name,
},
)
if not result:
return None
Expand Down Expand Up @@ -115,7 +128,9 @@ def test_android_hw_skipped(run_test):


def test_basic(run_test):
result = run_test({"attributes": {"unittest_variant": "os-integration"}}, "gecko")
result = run_test(
{"attributes": {"unittest_variant": "os-integration"}}, name="gecko"
)
assert result == {
"attributes": {"integration": "gecko"},
"dependencies": {"apply": "tc-admin-apply-staging"},
Expand All @@ -140,7 +155,7 @@ def test_docker_image(run_test):
"attributes": {"unittest_variant": "os-integration"},
"task": {"payload": {"image": {"taskId": "def"}}},
},
"gecko",
name="gecko",
)
assert result["dependencies"] == {
"apply": "tc-admin-apply-staging",
Expand Down Expand Up @@ -233,7 +248,7 @@ def test_private_artifact(run_test):
}
},
},
"gecko",
name="gecko",
)
assert result["dependencies"] == {
"apply": "tc-admin-apply-staging",
Expand Down
Loading