Skip to content

e2e testspec grouped actions - #5078

Open
williamvega wants to merge 27 commits into
masterfrom
feat/testspec-grouped-actions
Open

e2e testspec grouped actions#5078
williamvega wants to merge 27 commits into
masterfrom
feat/testspec-grouped-actions

Conversation

@williamvega

@williamvega williamvega commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

More e2e tests have bloated the testspec proto. The tests that send/watch events could use better grouping.

Similar to how is being done in #5018, it would make sense to group the action and its parameters together, instead of the top-level action/selection combination. In the proto, the api request itself can be referenced and additional information be wrapped in a message.

In that pr, a node-scoped action went from:

action: ACTION_CANCEL
selection: SELECTION_BY_NODE
nodePoolTag: node-ops
executor: local-cluster

to:

cancelOnNode:
  nodePoolTag: node-ops
  request:
    executor: local-cluster
    queues:
      - e2e-test-queue

This pr also converts all remaining tests to use the event itself defined in pkg/api directly instead of adding more event sub-fields to the testspec proto.

Fixes #5077

Signed-off-by: William Vega <williamvega1006@gmail.com>
…JobSet messages, needs a flag

Signed-off-by: William Vega <williamvega1006@gmail.com>
Signed-off-by: William Vega <williamvega1006@gmail.com>
Signed-off-by: William Vega <williamvega1006@gmail.com>
Signed-off-by: William Vega <williamvega1006@gmail.com>
Signed-off-by: William Vega <williamvega1006@gmail.com>
Signed-off-by: William Vega <williamvega1006@gmail.com>
@williamvega williamvega changed the title Feat/testspec grouped actions e2e testspec grouped actions Jul 30, 2026
Resolves testspec.proto conflicts by keeping both the grouped action
wrapper messages (cancel/cancel_job_set/preempt/reprioritize) from
this branch and the queue lifecycle config (queue_config/cancel_set)
from master, renumbering master's fields to 25/26 to avoid collision.

testspec.pb.go generated code accepted from master as-is; will be
regenerated locally to reflect the merged .proto.

Signed-off-by: William Vega <williamvega1006@gmail.com>
Drop the branch's original JobSetCancelConfig (request only) in favor
of the CancelSetConfig merged in from master, which also carries
assert_inactive_succeeds. Rename it to JobSetCancelConfig and rewire
the cancel_job_set field (26) to use it, retiring field 22.

Signed-off-by: William Vega <williamvega1006@gmail.com>
action, selection, new_priority, and preempt_reason (and the
ActionType/SelectionType enums) were superseded by the grouped
cancel/cancel_job_set/preempt/reprioritize wrappers and had no
remaining callers in Go code or test case YAML.

Signed-off-by: William Vega <williamvega1006@gmail.com>
Signed-off-by: William Vega <williamvega1006@gmail.com>
triggerEventExtractor did a bare map lookup on TriggerEvent with no
fallback for the empty-string case, contradicting its own doc comment
and breaking any test that omits triggerEvent while relying on the
documented default (e.g. jobset_cancel_by_set_1x5, jobset_cancel_inactive,
which use cancelJobSet with no explicit trigger).

Signed-off-by: William Vega <williamvega1006@gmail.com>
@armadaproject armadaproject deleted a comment from datadog-armadaproject Bot Aug 5, 2026
Signed-off-by: William Vega <williamvega1006@gmail.com>
Signed-off-by: William Vega <williamvega1006@gmail.com>
Signed-off-by: William Vega <williamvega1006@gmail.com>
Signed-off-by: William Vega <williamvega1006@gmail.com>
@williamvega
williamvega marked this pull request as ready for review August 5, 2026 18:41
@greptile-apps

greptile-apps Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR groups e2e test actions with their API request parameters and updates the test runner and YAML test specifications to consume the new shapes.

  • Replaces top-level action/selection fields with cancel, preempt, reprioritize, and job-set action configurations.
  • Centralizes action dispatch and node-name resolution in the test runner.
  • Regenerates the TestSpec protobuf bindings and migrates affected e2e test cases.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; the previously reported job-set preemption no-op is resolved by populating the request with every submitted job ID.

Important Files Changed

Filename Overview
internal/testsuite/testrunner.go Centralizes grouped-action detection and dispatch; the revised preemption path now supplies all submitted job IDs and resolves the prior no-op.
pkg/api/testspec.proto Replaces the top-level action and selection model with grouped request-specific action messages.
pkg/api/testspec.pb.go Regenerates Go protobuf types and serialization code for the new TestSpec schema.
internal/testsuite/queue/queue.go Treats a present queue configuration without an explicit setup block as a default queue setup.
testsuite/testcases/preemption/preempt_by_ids_1x5.yaml Migrates the multi-job preemption test to the grouped request format.

Reviews (11): Last reviewed commit: "Merge branch 'master' into feat/testspec..." | Re-trigger Greptile

Comment thread pkg/api/testspec.proto
Comment thread pkg/api/testspec.proto
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.

e2e tests: group testspec actions to reflect the event they are sending

2 participants