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

refactor CI testing of dart-services #2719

Merged
merged 14 commits into from
Nov 17, 2023
Merged

refactor CI testing of dart-services #2719

merged 14 commits into from
Nov 17, 2023

Conversation

devoncarew
Copy link
Member

@devoncarew devoncarew commented Nov 16, 2023

This PR has grown a bit but includes some important fixes.


  • I’ve reviewed the contributor guide and applied the relevant portions to this PR.
Contribution guidelines:

Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.

runs-on: ubuntu-latest
defaults:
run:
working-directory: pkgs/dart_services/
strategy:
fail-fast: false
matrix:
sdk: [stable, beta, main]
sdk: [stable, beta, master] # main
Copy link
Member Author

Choose a reason for hiding this comment

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

subosito/flutter-action doesn't seem to support main

Copy link
Member Author

Choose a reason for hiding this comment

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

@devoncarew
Copy link
Member Author

With this PR, both beta and main are failing, but they're both failing the same way (due to #2718).

We now run build-storage-artifacts nightly in order to find regressions sooner.

We're working around an issue with the flutter sdk's version file not being created for sdks on the main branch.

In addition, it doesn't look like subosito/flutter-action is happy with specifying the main branch (we use master instead).

@devoncarew
Copy link
Member Author

beta is fixed now, and main is past the issue with the moved flutter framework dill file.

master is now failing on a calls to ui.webOnlyInitializePlatform() - it looks like that's been removed (and perhaps is now a call to ui_web.bootstrapEngine()?). In any case, I think what's here is now worth review - it includes several fixes. We can work on additional fixes to the main channel in future PRs.

@devoncarew devoncarew marked this pull request as draft November 17, 2023 18:45
@devoncarew devoncarew marked this pull request as ready for review November 17, 2023 19:12
Map<String, dynamic> _callFlutterVersion() {
// Note that we try twice here as the 'flutter --version --machine' command
// can (erroneously) emit non-json text to stdout (for example, an initial
// analytics disclaimer).
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we need to run flutter config --no-analytics and dart --disable-analytics somewhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

flutter config --no-analytics is a good call. But flutter --version --machine - which emits json - should never emit cruft to stdout; the output needs to be parseable.

@devoncarew
Copy link
Member Author

Note that there is a test failure for main related to firebase. This is worth landing irrespective.

@devoncarew devoncarew merged commit 5e300f9 into main Nov 17, 2023
10 checks passed
@devoncarew devoncarew deleted the update_services_ci branch November 17, 2023 19:22
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.

unable to deploy the dartpad backend for the beta and main channels
2 participants