-
Notifications
You must be signed in to change notification settings - Fork 362
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
[cronet_http] Enables CI for cronet_http_embedded
#1070
Changes from 17 commits
4506ff8
91e5ba6
aee5e9a
f868580
e51d816
c7bcdbf
58cd8db
8fe848d
0637349
c6ea3b4
368be83
f55683d
e3a9c14
2c25829
261fd37
1d379e6
05d63a8
702ad03
2efc135
6e3d84b
48dfa0e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,10 +6,12 @@ on: | |
- main | ||
- master | ||
paths: | ||
- '.github/workflows/**' | ||
- 'pkgs/cronet_http/**' | ||
- 'pkgs/http_client_conformance_tests/**' | ||
pull_request: | ||
paths: | ||
- '.github/workflows/**' | ||
- 'pkgs/cronet_http/**' | ||
- 'pkgs/http_client_conformance_tests/**' | ||
schedule: | ||
|
@@ -19,48 +21,45 @@ env: | |
PUB_ENVIRONMENT: bot.github | ||
|
||
jobs: | ||
analyze: | ||
name: Lint and static analysis | ||
runs-on: ubuntu-latest | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why did you change this to macos? I think that there is more linux machine availability. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are unknown timeouts when running on the Ubuntu runner. https://github.com/dart-lang/http/actions/runs/7270023551/job/19808487300?pr=1070 |
||
defaults: | ||
run: | ||
working-directory: pkgs/cronet_http | ||
verify: | ||
name: Format & Analyze & Test | ||
runs-on: macos-latest | ||
strategy: | ||
matrix: | ||
package: ['cronet_http', 'cronet_http_embedded'] | ||
steps: | ||
- uses: actions/checkout@v4 | ||
- uses: actions/setup-java@v4 | ||
with: | ||
distribution: 'zulu' | ||
java-version: '17' | ||
- uses: subosito/flutter-action@v2 | ||
with: | ||
# TODO: Change to 'stable' when a release version of flutter | ||
# pins version 1.21.1 or later of 'package:test' | ||
channel: 'master' | ||
channel: 'stable' | ||
- name: Make cronet_http_embedded copy | ||
if: ${{ matrix.package == 'cronet_http_embedded' }} | ||
run: | | ||
cp -r pkgs/cronet_http pkgs/cronet_http_embedded | ||
cd pkgs/cronet_http_embedded | ||
flutter pub get && dart tool/prepare_for_embedded.dart | ||
- id: install | ||
name: Install dependencies | ||
working-directory: 'pkgs/${{ matrix.package }}' | ||
run: flutter pub get | ||
- name: Check formatting | ||
working-directory: 'pkgs/${{ matrix.package }}' | ||
run: dart format --output=none --set-exit-if-changed . | ||
if: always() && steps.install.outcome == 'success' | ||
- name: Analyze code | ||
working-directory: 'pkgs/${{ matrix.package }}' | ||
run: flutter analyze --fatal-infos | ||
if: always() && steps.install.outcome == 'success' | ||
|
||
test: | ||
# Test package:cupertino_http use flutter integration tests. | ||
needs: analyze | ||
name: "Build and test" | ||
runs-on: macos-latest | ||
steps: | ||
- uses: actions/checkout@v4 | ||
- uses: actions/setup-java@v4 | ||
with: | ||
distribution: 'zulu' | ||
java-version: '17' | ||
- uses: subosito/flutter-action@v2 | ||
with: | ||
channel: 'stable' | ||
- name: Run tests | ||
uses: reactivecircus/android-emulator-runner@v2 | ||
if: always() && steps.install.outcome == 'success' | ||
with: | ||
api-level: 28 | ||
target: playstore | ||
target: ${{ matrix.package == 'cronet_http_embedded' && 'default' || 'playstore' }} | ||
arch: x86_64 | ||
profile: pixel | ||
script: cd ./pkgs/cronet_http/example && flutter test --timeout=1200s integration_test/ | ||
script: cd 'pkgs/${{ matrix.package }}/example' && flutter test --timeout=1500s integration_test/ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I must misunderstand how this works - how can the tests work if the package imports inside the tests are not updated? I mean, isn't the package test code still doing: import 'package:cronet_http/cronet_http.dart'; But the tests pass so I'm confused ;-) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It doesn't change in the latest commit. I guess there are some other problems with the Ubuntu setup. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,14 +2,14 @@ group 'io.flutter.plugins.cronet_http' | |
version '1.0-SNAPSHOT' | ||
|
||
buildscript { | ||
ext.kotlin_version = '1.6.10' | ||
ext.kotlin_version = '1.7.21' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All of these types of upgrades should be done in a separate PR, no? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm planning to reorg workflows and examples for the repo in further separate PRs. These are necessary for the current setup. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, feel free to set these to any reasonable values. |
||
repositories { | ||
google() | ||
mavenCentral() | ||
} | ||
|
||
dependencies { | ||
classpath 'com.android.tools.build:gradle:7.1.2' | ||
classpath 'com.android.tools.build:gradle:7.4.2' | ||
classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlin_version" | ||
} | ||
} | ||
|
@@ -25,6 +25,11 @@ apply plugin: 'com.android.library' | |
apply plugin: 'kotlin-android' | ||
|
||
android { | ||
// Conditional for compatibility with AGP <4.2. | ||
if (project.android.hasProperty("namespace")) { | ||
namespace 'io.flutter.plugins.cronet_http' | ||
} | ||
|
||
compileSdkVersion 31 | ||
|
||
compileOptions { | ||
|
@@ -41,7 +46,7 @@ android { | |
} | ||
|
||
defaultConfig { | ||
minSdkVersion 16 | ||
minSdkVersion 19 | ||
} | ||
|
||
defaultConfig { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
<manifest xmlns:android="http://schemas.android.com/apk/res/android" | ||
package="com.example.cronet_http_example"> | ||
package="io.flutter.cronet_http_example"> | ||
<uses-permission android:name="android.permission.INTERNET"/> | ||
</manifest> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
<manifest xmlns:android="http://schemas.android.com/apk/res/android" | ||
package="com.example.cronet_http_example"> | ||
package="io.flutter.cronet_http_example"> | ||
<uses-permission android:name="android.permission.INTERNET"/> | ||
</manifest> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe only trigger on changes to this file? So updating the monorepo spec doesn't trigger a 20 minute presubmit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah sure. Will update these in another PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I already updated this.