Skip to content

Conversation

@Eijebong
Copy link
Contributor

The documentation already says You can generate actions.json locally by running taskgraph actions. however that command was never ported over from gecko's ./mach taskgraph actions

@Eijebong Eijebong changed the title Port taskgraph action from gecko_taskgraph feat: port taskgraph action from gecko_taskgraph Feb 14, 2025
Copy link
Collaborator

@ahal ahal left a comment

Choose a reason for hiding this comment

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

Thanks!

@Eijebong
Copy link
Contributor Author

Marking this back as a draft as after talking to @hneiva about it, I discovered that there were indeed a few tests in test_main where the CLI was tested so I started writing some.

@Eijebong Eijebong marked this pull request as draft February 21, 2025 16:46
The documentation already says `You can generate actions.json locally by
running taskgraph actions.` however that command was never ported over
from gecko's `./mach taskgraph actions`
To be able to test without relying on actions that are always generated
too much, I monkeypatch `registry._load` so it simply doesn't import the
modules registering those actions.

Adding that test found that parameters should be parsed with
`strict=False` to match other CLI actions. The return value isn't
strictly necessary but I added it to be consistent with other functions
in the CLI.
@Eijebong Eijebong marked this pull request as ready for review February 25, 2025 12:01
@Eijebong Eijebong requested a review from ahal February 25, 2025 12:01
@Eijebong
Copy link
Contributor Author

Ah great, something is leaving the test env dirty.

uv run -v pytest ./test/test_main.py passes but uv run -v pytest doesn't

@Eijebong
Copy link
Contributor Author

Ah yeah, I see

registry.sanity_check_task_scope(callback, parameters, graph_config={})
-> https://github.com/taskcluster/taskgraph/blob/main/src/taskgraph/actions/registry.py#L278 -> https://github.com/taskcluster/taskgraph/blob/main/src/taskgraph/actions/registry.py#L361 -> https://github.com/taskcluster/taskgraph/blob/main/src/taskgraph/actions/registry.py#L349

The modules declaring the actions get imported, the actions registered. Later when the second test runs, it mocks the actions list from the registry, then calls things to reload the actions. The modules being already imported by the previous test, it's a no-op, actions don't get re-registered, test fails. Fun

Otherwise a previous test might have imported the actions modules
already, making `registry._load` a no-op and failing the test
Copy link
Contributor

@hneiva hneiva left a comment

Choose a reason for hiding this comment

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

LGTM!

@Eijebong Eijebong merged commit 63ec732 into taskcluster:main Feb 26, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants