Skip to content

Remove support tier infrastructure in favor of caller-requested platform testing.#46

Merged
str4d merged 5 commits intomainfrom
remove_support_tiers
Mar 10, 2026
Merged

Remove support tier infrastructure in favor of caller-requested platform testing.#46
str4d merged 5 commits intomainfrom
remove_support_tiers

Conversation

@nuttycom
Copy link
Contributor

@nuttycom nuttycom commented Mar 6, 2026

No description provided.

Copy link
Collaborator

@str4d str4d left a comment

Choose a reason for hiding this comment

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

Reviewed a78aad0

```

For each new project, either:
- Create project-specific composite actions, or
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a bad suggestion. The entire point of creating start-interop and finish-interop was to avoid duplicated project-specific logic in the main ci.yml. Rework this documentation to remove this idea entirely.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm moving this documentation out of this PR.

Comment on lines +293 to +294
- Generalize the existing actions to accept the target owner, repository, and
secret names as inputs.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is the wrong way to describe the correct action. The composite actions are already generalized (as used in ci.yml); they just need additional inputs to be added to them, and corresponding additional steps (as done in #29). Rewird this documentation to describe the action appropriately.

Comment on lines +296 to +298
Each job that should report status back must call `start-interop` at the
beginning and `finish-interop` (with `if: always()`) at the end.

Copy link
Collaborator

Choose a reason for hiding this comment

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

After my previous comments are addressed, this paragraph is superfluous (as deciding which jobs report status back is orthogonal to configuring the cross-repo interop).

Suggested change
Each job that should report status back must call `start-interop` at the
beginning and `finish-interop` (with `if: always()`) at the end.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I did not closely review the GitHub App parts of this file.

Copy link
Contributor

Choose a reason for hiding this comment

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

The app architecture described here (two separate apps: one for dispatch, one for status-reporting) differs from what's already been provisioned by @gustavovalverde in #29 (comment). The set up there was a single enterprise app (multi-org-z3-integration-testing) installed across both orgs, with Z3_APP_ID and Z3_APP_PRIVATE_KEY stored as secrets in both zcash/integration-tests and ZcashFoundation/zebra handling both dispatch and status-reporting directions with one app.

That's also what ZcashFoundation/zebra#10372 is supposed to use.

I see the two-app model might have security advantages. We need to confirm what is what we are going to do so we can request the exact set up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, I was thinking about the integration with the Zingo repositories when I made this change to the interop definition. When I first set up the zcash github app, it was easy to install it on both repositories because they both belonged to the same org, but the decision to use a single app wasn't a carefully considered one. My plan has been to move to the two-app setup internal to the zcash org as well, but I'm open to discussion as to which approach is better.

nuttycom and others added 3 commits March 9, 2026 14:42
Replace the tier numbering system (inherited from zcashd) with a simple
`required` boolean on each platform matrix entry. Individual component
repositories should define their own platform support policies; this
repo only needs to know which platforms must pass vs. which are
best-effort.

- Replace `tier: N` with `required: true/false` in CI matrix
- Update job names to show platform name without tier
- Update ci-status.yml required-pass regex for new job names

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When CI is triggered via repository_dispatch (e.g. zallet-interop-request),
callers can now specify which platforms to run by including a `platforms`
array in the client_payload. For example:

  {"sha": "abc123", "platforms": ["ubuntu-22.04"]}

When `platforms` is absent, all configured platforms run (preserving
existing behavior for PR and push triggers).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The rationale for this change is that while pull requests to the
integration testing repository should run against the full suite,
including all the tests that are not currently expected to pass,
for external parties requesting integration testing the primary
concern is that changes do not break existing expected-to-pass tests.
@nuttycom nuttycom force-pushed the remove_support_tiers branch from a78aad0 to ed149de Compare March 9, 2026 21:48
… reporting

- Replace repeated `matrix.required && ' (required)' || ''` expressions
  with a `matrix.required_suffix` field computed once in filter-platforms.
- Remove unnecessary jq round-trip in the else branch of filter-platforms
  (jq is now justified by the required_suffix computation).
- Report error statuses on the requesting SHA for each requested platform
  that is not supported by this repository's configuration.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@nuttycom
Copy link
Contributor Author

nuttycom commented Mar 9, 2026

force-pushed to address review comments then added c9126e1 to address further comments from review.

Copy link
Collaborator

@str4d str4d left a comment

Choose a reason for hiding this comment

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

Reviewed c9126e1

@nuttycom nuttycom requested a review from str4d March 9, 2026 23:54
num_old_tests = len(rpc_tests.BASE_SCRIPTS + rpc_tests.ZMQ_SCRIPTS)
num_new_tests = len(rpc_tests.NEW_SCRIPTS)
old_shards = (num_old_tests * SHARDS) // (num_old_tests + num_new_tests)

Check notice

Code scanning / zizmor

code injection via template expansion Note

code injection via template expansion
@nuttycom nuttycom force-pushed the remove_support_tiers branch from 27e5eaf to 16ca14a Compare March 10, 2026 00:02
Copy link
Collaborator

@str4d str4d left a comment

Choose a reason for hiding this comment

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

utACK 16ca14a

Comment on lines +13 to +16
outputs:
zallet-app-token:
description: "Access token for updating the Zallet pending status"
value: ${{ steps.zallet-app-token.outputs.token }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unused for now, but meh it can stay.

@str4d str4d merged commit 84e2107 into main Mar 10, 2026
22 of 38 checks passed
@str4d str4d deleted the remove_support_tiers branch March 10, 2026 01:30
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