Skip to content

GHA to run sage and check things work#86

Merged
joellabes merged 17 commits intomainfrom
add-gha-for-regression-testing
Dec 10, 2025
Merged

GHA to run sage and check things work#86
joellabes merged 17 commits intomainfrom
add-gha-for-regression-testing

Conversation

@joellabes
Copy link
Collaborator

@joellabes joellabes commented Dec 10, 2025

Make sure that changes to a task, shared project, etc, don't cause an experiment to fail when run against the sage. And, check that all experiments (except analytics_engineering001 which is designed to test a no-op) fail when run with no agent

Running each project type with a single concurrent run to avoid #76's race conditions, but this can be changed once that’s solved
‼️ incorporates change set from #88 - review that first ‼️

@joellabes joellabes marked this pull request as draft December 10, 2025 02:37
@joellabes joellabes marked this pull request as ready for review December 10, 2025 03:00
@joellabes joellabes requested a review from bstancil December 10, 2025 03:02
@joellabes
Copy link
Collaborator Author

ok this is ready to review now the race condition is sorted @bstancil

@joellabes joellabes enabled auto-merge (squash) December 10, 2025 21:59
Copy link
Collaborator

@bstancil bstancil left a comment

Choose a reason for hiding this comment

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

Ok, I'm into this conceptually, but are there easy ways to bypass it (or ways to make it so that we elect into running it?) These things can take some time to run, so blocking every PR on it feels very heavy?

Y'all have a lot more experience / opinions about this than I do, so i'll take your judgement on it. But if the test suite grows, this could become vey cumbersome.

@joellabes
Copy link
Collaborator Author

At the moment the time between "PR open" and "PR approved" is normally multiple hours, and the suite finishes in ~10 minutes, so I think we've got a fair bit of headroom before it becomes the limiting factor. It's also skipping markdown changes etc so quick cleanups shouldn't be an issue.

You can imagine a Slim CI setup which only runs modified tests, I couldn't be bothered faffing with that right now but the good naming conventions should make that reasonably approachable when the time comes.

Enabling auto-merge also means that a PR can be human reviewed and approved pending successful regression tests, without having to remember to come back and do it later.

So I think we should be ok for the foreseeable!

Copy link
Collaborator

@bstancil bstancil left a comment

Choose a reason for hiding this comment

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

aight, let's do it. and if we need to change it, that's easy enough to do when we want to.

@joellabes joellabes merged commit 558fa63 into main Dec 10, 2025
5 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