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

also test production drivers #224

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

also test production drivers #224

wants to merge 5 commits into from

Conversation

matthiasdiener
Copy link
Collaborator

No description provided.


# Test production drivers
source .ci_support/production_testing_env.sh
source .ci_support/merge-install-production-branch.sh .
Copy link
Collaborator

@MTCam MTCam Mar 2, 2022

Choose a reason for hiding this comment

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

Down inside there there is a pip install -r requirements.txt after merging with production. Will that mess stuff up, or switch the grudge branch? Edit: Could just sed out the grudge part of it?

Copy link
Owner

@inducer inducer left a comment

Choose a reason for hiding this comment

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

I'm not sure I'm sold. I'm happy to keep mirgecom main working. Production, by definition, depends on a lot of unmerged stuff (including, potentially, grudge changes), so I feel this would be a high-maintenance proposition.


if test "$DOWNSTREAM_PROJECT" = "mirgecom"; then
# Test main branch
examples/run_examples.sh ./examples

# Test production drivers
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe this should be a separate CI job? mirgecom is already the longest job in grudge, I'm reluctant to make CI turnaround even more sluggish.

Copy link
Collaborator

@MTCam MTCam Mar 2, 2022

Choose a reason for hiding this comment

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

Then maybe drop the downstream pytest (which takes a long time)? Running the examples (typically done within 10 minutes) will be enough and much faster. The examples won't work if MIRGE-Com is broken, and they will also fail if they get anything outside the accepted solution range.

Edit: I mean to suggest downstream examples is enough, full stop. No production testing either, as you suggested.

@inducer
Copy link
Owner

inducer commented Mar 2, 2022

I'm not sure I'm sold. I'm happy to keep mirgecom main working.

As an added bonus, this provides an incentive to not let production and main drift too far apart.

@MTCam
Copy link
Collaborator

MTCam commented Mar 2, 2022

I'm not sure I'm sold. I'm happy to keep mirgecom main working.

As an added bonus, this provides an incentive to not let production and main drift too far apart.

I don't think neglecting production testing in grudge CI will affect the amount of drift between mirgecom@production and mirgecom@main. That said, I do not disagree with your notion leave it out.

Base automatically changed from ci_mirgecom_examples to main March 3, 2022 20:50
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