Split tests by modules in Github Actions #558
Conversation
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - name: Setup test environment | ||
| uses: ./.github/actions/setup_test_action | ||
| timeout-minutes: 10 | ||
| - name: Run JS Tests | ||
| run: ./gradlew cleanTest jsTest | ||
| run: ./gradlew ${{ matrix.gradle_tasks }} | ||
| - name: Upload JS test artifact | ||
| uses: actions/upload-artifact@v4 |
There was a problem hiding this comment.
All artifacts should have a unique name, so it should derive the name of the artifact from the gradle task. E.g "JS firebase-firestore Firebase Debug Log"
| "firebase-analytics" | ||
| ).contains(subProject.name).not() | ||
| }.map { subProject -> | ||
| "${subProject.path}:iosSimulatorArm64Test" |
There was a problem hiding this comment.
This should check for the CPU architecture running the gradle script. For X86-64 CPUs it should be using iosX64Test
firebase-storage/build.gradle.kts
Outdated
| @@ -143,24 +143,6 @@ kotlin { | |||
| } | |||
| } | |||
|
|
|||
| if (project.property("firebase-storage.skipIosTests") == "true") { | |||
There was a problem hiding this comment.
Is there anywhere these tests are still disabled when you run them locally. While the gradle script is fine for generating the CI matrix, we should still be able to run ./gradlew testIosArmSimulator64 without issue on a local checkout
There was a problem hiding this comment.
I propose two ways for running tests locally:
-
create custom Gradle tasks (like
devIosTests,devJsTests,devJvmTests) in root build.gradle.kts file. This tasks will use the EmulatorJobsMatrix.kt logic. As result many build.gradle.kts files will have less code. -
EmulatorJobsMatrix.kt logic use the
gradle.propertiesfile for excluding jobs from matrix.
@Daeda88 What do you prefer for running tests locally?
There was a problem hiding this comment.
I think it should be the latter. Making custom gradle tasks to get back functionality that is there by default on gradle feels a bit weird to me.
You are right on having many build gradle scripts that do the same thing, but I think a better long term solution for that is to make a custom gradle plugin that does all the recurring setup. Not that I;m saying you should implement that mind you, just that if we want to reduce code, there is a better way.
There was a problem hiding this comment.
I think it should be the latter. Making custom gradle tasks to get back functionality that is there by default on gradle feels a bit weird to me.
Agreed, the default test tasks should work as people expect
| }.map { subProject -> | ||
| when (val osArch = System.getProperty("os.arch")) { | ||
| "x86", "i386", "ia-32", "i686", | ||
| "x86_64", "amd64", "x64", "x86-64" -> "${subProject.path}:iosSimulatorX64Test" |
There was a problem hiding this comment.
Task is called iosX64Test for X64 and iosX86Test for x86 machines. No Simulator in there. Since iphones were always Arm based, there is a distinction between SimulatorArm (a simulator) and Arm (an actual iphone) that is not required for Intel architecture
@siarhei-luskanau do you know why these get stuck on waiting for status to be reported?
|
@nbransby I have no idea. Looks like it is related the matrix jobs |
|
Is there a way to fix that? |
Probably issue is related the brunch rules https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-protected-branches/about-protected-branches#about-branch-protection-rules |
|
I don't believe so as it works on other PRs fine - example #504 |
I have found similar issue with |
|
Ok thanks for the extra info, I've fixed that now. There is one failing test which I don't think has anything to do with these changes but would be good to get fixed before merging in: Not sure why there is network connection issues with the ios simulator, could it be to do with the Firebase Local Emulator Suite setup? |
It looks like the flaky fails, I didn't changed the Firebase related code. Please re-launch the failed jobs. |
No description provided.