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

CI: Prevent merging algorand-sdk-testing PRs that break SDKs #165

Open
michaeldiamant opened this issue Mar 17, 2022 · 0 comments
Open

CI: Prevent merging algorand-sdk-testing PRs that break SDKs #165

michaeldiamant opened this issue Mar 17, 2022 · 0 comments
Labels

Comments

@michaeldiamant
Copy link
Contributor

Problem

Merging algorand-sdk-testing in a state that breaks downstream SDKs in master shouldn’t happen. But, in practice, it happens without mal-intent.

Background:

  • Since there’s no build check in algorand-sdk-testing, it’s possible to unintentionally merge test changes that break SDKs.
  • Since there’s no time-based SDK build process (e.g. nightly), no one discovers a broken SDK build until someone tries to build the project. It’s difficult to correlate a build failure with root problem cause.
  • Motivating example:
  • With the as is conventions, the encouraged way to make changes without breaking SDKs is to make 2-step changes as described below.
    • In an algorand-sdk-testing PR, duplicate the test in-question. Assign a unique annotation.
    • Merge the PR. SDKs are safe because:
      • New test is not run until the annotation is configured.
      • All other tests are unchanged.
    • After making + merging changes to all SDKs, remove the test that was duplicated.

Solution

Unclear - From group discussion, at least a couple of paths exist. Bolded proposal is preferred though more discussion is needed.

  • Modify algorand-sdk-testing build process to test downstream SDKs. Implies failing the build when a downstream SDK's tests fail.
    • There's a circular dependency between algorand-sdk-testing and SDKs that complicates the checks.
    • It's possible to imagine a build process like:
      • algorand-sdk-testing build looks for SDK branch names matching algorand-sdk-testing branch. If none found, use the SDK repo's default branch.
      • Run integration tests on all SDKs.
    • Concerns:
      • Some algorand-sdk-testing changes have no downstream consequences. The process must work for such scenarios.
      • The proposed process feels heavyweight. It likely results in a long build time.
  • Make SDKs depend on a tag of algorand-sdk-testing instead of master. Implies implementing a process to ensure SDK repos update algorand-sdk-testing on release.
    • Allows for looser coupling among SDK changes. Should support retaining a simpler build process.
    • It's possible to imagine starting with a manual process for keeping tags in-sync. In principle, stories won't be closed until corresponding SDK changes complete.
    • Constraint: All SDKs provide the same approach for configuring algorand-sdk-testing git tag/branch without requiring hand-modified shell script modifications.

Stopgap measures until taking on an approach from above:

  • Setup a nightly build for each SDK.
    • If an algorand-sdk-testing PR breaks an SDK, we'll know within 24 hours rather than when someone else attempts to build.
    • The process feels straightforward to implement. And provides more visibility than we have today.
    • Once a more principled approach is implemented, we can remove the nightly builds.

Dependencies

N/A

Urgency

TBD

@michaeldiamant michaeldiamant added the new-feature-request Feature request that needs triage label Mar 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant