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: channel compatibility workflow #123

Merged
merged 27 commits into from
Sep 18, 2024
Merged

ci: channel compatibility workflow #123

merged 27 commits into from
Sep 18, 2024

Conversation

btrautmann
Copy link
Contributor

@btrautmann btrautmann commented Sep 13, 2024

Description

This PR adds a workflow that will eventually run daily to check alchemist's compatibility with the current latest stable and beta versions of the framework.

Note: I'm not enabling the CRON schedule right now because there are actually a couple failing tests on stable and beta channels. It looks like there were some more color changes introduced at some point that cause large pixel diffs. I'm going to try and tackle #120 and then enable this.

The idea of this workflow is to get advanced notice of changes that would "break" consumers of alchemist (for a number of reasons). One recent example is that in upgrading our min version constraint to 3.16.0 we ran against the fact that this was the first major version of Flutter that enabled material 3 by default. Changes trickled down through Alchemist and caused golden failures for consumers, even when they had disabled material 3 in their projects. We should aim to be material agnostic and, all else equal, test golden images generated by alchemist should be compatible with master golden images generated on different versions of the framework. This may not be possible in all cases, but this workflow will at least give us advanced notice so we can decide what is alchemist's fault and what is Flutter's fault.

When we encounter a failure, those in the #mobile-oss-coordination channel will see a message similar to this:

Screenshot 2024-09-13 at 16 01 50

Closes #119.
Fixes #105.

Type of Change

BREAKING CHANGE: This technically introduces a breaking change in that it removes a couple Paddings to reduce Alchemist's impact on the resulting goldens.

  • ✨ New feature (non-breaking change which adds functionality)
  • 🛠️ Bug fix (non-breaking change which fixes an issue)
  • ❌ Breaking change (fix or feature that would cause existing functionality to change)
  • 🧹 Code refactor
  • ✅ Build configuration change
  • 📝 Documentation
  • 🗑️ Chore

@btrautmann btrautmann changed the title Bt/compat job ci: channel compatibility workflow Sep 13, 2024
jeroen-meijer
jeroen-meijer previously approved these changes Sep 16, 2024
Copy link
Collaborator

@jeroen-meijer jeroen-meijer left a comment

Choose a reason for hiding this comment

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

Nice!

:shipit:

Copy link

codecov bot commented Sep 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (cd09414) to head (2755a87).
Report is 7 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #123   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           13        13           
  Lines          562       555    -7     
=========================================
- Hits           562       555    -7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Kirpal
Kirpal previously approved these changes Sep 18, 2024
Copy link
Member

@samandmoore samandmoore left a comment

Choose a reason for hiding this comment

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

lgtm!

@btrautmann btrautmann merged commit 5139917 into main Sep 18, 2024
7 checks passed
@btrautmann btrautmann deleted the bt/compat-job branch September 18, 2024 14:01
@@ -263,11 +263,8 @@ class FlutterGoldenTestAdapter extends GoldenTestAdapter {
maxWidth: constraints.maxWidth,
maxHeight: constraints.maxHeight,
child: Center(
child: Padding(
Copy link
Contributor

Choose a reason for hiding this comment

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

@btrautmann what is the reasoning behind this removal of the Padding?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Screenshot 2024-10-03 at 09 17 57

See #105

I didn't think it would make sense to maintain a configuration option so I removed the padding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants