Skip to content

PTS improvements #566

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.

Sign up for GitHub

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

Merged
ribafish merged 8 commits into from
Aug 29, 2023
Merged

PTS improvements #566

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f5b2cc8
Enabled PTS by default and set the FAST profile for `testAndroid${ver…
ribafish Aug 18, 2023
68ac58e
Set PTS to disabled by default when running manually (workflow trigge…
ribafish Aug 21, 2023
15af1aa
Removed non user faacing changes from changes.md
ribafish Aug 21, 2023
4929580
Reworked PTS mode to be set in ENV parts of the build-verification.ym…
ribafish Aug 22, 2023
277464d
Cleanup
ribafish Aug 22, 2023
c2239b6
Merge branch 'main' into gk/ptsImprovements
ribafish Aug 22, 2023
21ffe45
Removed the workflow_dispatch.input, which was used to disable PTS by…
ribafish Aug 28, 2023
7d2bc51
Merge branch 'main' into gk/ptsImprovements
ribafish Aug 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions .github/workflows/build-verification.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ on:
push:
branches: [ main, citest ]
pull_request:
branches: [ main ]
workflow_dispatch:

jobs:
Expand All @@ -16,7 +15,7 @@ jobs:
- id: setup-matrix
run: echo "matrix=$(jq -cM '.supportedVersions | keys' src/main/resources/versions.json | sed -e 's/\./_/g' -e 's/-/_/g')" >> $GITHUB_OUTPUT
- name: debug
run: echo ${{ steps.setup-matrix.outputs.matrix }}
run: echo "matrix=${{ steps.setup-matrix.outputs.matrix }}"
Copy link
Contributor

@runningcode runningcode Aug 28, 2023

Choose a reason for hiding this comment

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

just curious, why this change?

Copy link
Member Author

@ribafish ribafish Aug 28, 2023

Choose a reason for hiding this comment

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

Oh, it's still left from the first variant where I set the PTS mode in that job, but I still left it in as it's more verbose when looking at logs. I can remove this change if wanted.

Copy link
Contributor

@runningcode runningcode Aug 28, 2023

Choose a reason for hiding this comment

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

let's leave it how it originally was to simplify and reduce the changeset.

outputs:
matrix: ${{ steps.setup-matrix.outputs.matrix }}

Expand Down Expand Up @@ -48,10 +47,15 @@ jobs:
- name: Set up Gradle
uses: gradle/gradle-build-action@v2
- name: Run unit tests
run: ./gradlew test -x signPluginMavenPublication -x signAndroidCacheFixPluginPluginMarkerMavenPublication -Porg.gradle.java.installations.auto-download=false
run: >-
./gradlew test
-x signPluginMavenPublication
-x signAndroidCacheFixPluginPluginMarkerMavenPublication
-Porg.gradle.java.installations.auto-download=false
-Dpts.mode=$PTS_MODE
env:
GRADLE_ENTERPRISE_ACCESS_KEY: ${{ secrets.GE_SOLUTIONS_ACCESS_TOKEN }}
ORG_GRADLE_PROJECT_isPTSEnabled: ${{ github.ref_name != 'main' }}
Copy link
Contributor

@runningcode runningcode Aug 22, 2023

Choose a reason for hiding this comment

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

Is it possible to use variable here to set the remaining tests mode inline instead of having an action to set it?

something like pts.mode: if ${{ github.ref_name == 'main' }}; then 'REMAINING_TESTS'; else 'RELEVANT_TESTS' and then pass that value?

Copy link
Member Author

@ribafish ribafish Aug 22, 2023

Choose a reason for hiding this comment

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

if statements can't be used in the ENV part of actions, but there are some tertiary workarounds, one of which I've applied. It does need to be duplicated on both test jobs though...

Copy link
Contributor

@runningcode runningcode Aug 22, 2023

Choose a reason for hiding this comment

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

Thanks for looking in to this. While there is some duplication in this solution, i prefer the simplicity of it. It also makes it easier to use as an example for customers.

PTS_MODE: "${{ github.ref_name == 'main' && 'REMAINING_TESTS' || 'RELEVANT_TESTS' }}"

android_version_tests:
name: Android version tests
Expand Down Expand Up @@ -99,7 +103,12 @@ jobs:
- name: Set up Gradle
uses: gradle/gradle-build-action@v2
- name: Run integration tests
run: ./gradlew testAndroid${{ matrix.versions }} -x signPluginMavenPublication -x signAndroidCacheFixPluginPluginMarkerMavenPublication -Porg.gradle.java.installations.auto-download=false
run: >-
./gradlew testAndroid${{ matrix.versions }}
-x signPluginMavenPublication
-x signAndroidCacheFixPluginPluginMarkerMavenPublication
-Porg.gradle.java.installations.auto-download=false
-Dpts.mode=$PTS_MODE
env:
GRADLE_ENTERPRISE_ACCESS_KEY: ${{ secrets.GE_SOLUTIONS_ACCESS_TOKEN }}
ORG_GRADLE_PROJECT_isPTSEnabled: ${{ github.ref_name != 'main' }}
PTS_MODE: "${{ github.ref_name == 'main' && 'REMAINING_TESTS' || 'RELEVANT_TESTS' }}"
15 changes: 14 additions & 1 deletion build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import com.gradle.enterprise.gradleplugin.testretry.retry
import com.gradle.enterprise.gradleplugin.testselection.PredictiveTestSelectionProfile.FAST
import com.gradle.enterprise.gradleplugin.testselection.PredictiveTestSelectionProfile.CONSERVATIVE
import com.gradle.enterprise.gradleplugin.testselection.PredictiveTestSelectionProfile.STANDARD
import groovy.json.JsonSlurper

plugins {
Expand Down Expand Up @@ -113,7 +116,13 @@ tasks.withType<Test>().configureEach {
}

predictiveSelection {
enabled = providers.gradleProperty("isPTSEnabled").map { it != "false" }.orElse(false)
enabled = providers.gradleProperty("isPTSEnabled").map { it != "false" }.orElse(true)
}
}

tasks.test {
predictiveSelection {
profile = STANDARD
}
}

Expand All @@ -135,6 +144,10 @@ getSupportedVersions().keys.forEach { androidVersion ->
languageVersion = JavaLanguageVersion.of(17)
}
}

predictiveSelection {
profile = FAST
}
}

tasks.check {
Expand Down
2 changes: 1 addition & 1 deletion gradle.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@ org.gradle.configuration-cache=true
org.gradle.jvmargs=-Duser.language=en -Duser.country=US -Dfile.encoding=UTF-8
org.gradle.kotlin.dsl.allWarningsAsErrors=true

isPTSEnabled=false
isPTSEnabled=true
2 changes: 1 addition & 1 deletion release/changes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
- [NEW] Run tests against AGP 8.3.0-alpha01
- [NEW] Run tests against AGP 8.2.0-beta01
- [NEW] Run tests against AGP 8.1.1
Copy link
Contributor

@runningcode runningcode Aug 22, 2023

Choose a reason for hiding this comment

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

why is this showing a diff? is there a whitespace change?

Copy link
Member Author

@ribafish ribafish Aug 22, 2023

Choose a reason for hiding this comment

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

Yeah, I think it was without the newline at the end before. It's weird though, as I resolved it to the main branch variant of it when merging..

- [NEW] Run tests against AGP 8.1.1