nowinandroid icon indicating copy to clipboard operation
nowinandroid copied to clipboard

[FR]: Add build workflow for app-nia-catalog

Open arriolac opened this issue 2 years ago • 3 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

Describe the problem

The current Build.yaml workflow is specific to building and testing the app module, however, there are more than 1 apps in the project.

Describe the solution

An additional build/test workflow should be added to also build/test app-nia-catalog

Additional context

N/A

Code of Conduct

  • [X] I agree to follow this project's Code of Conduct

arriolac avatar Jun 03 '22 19:06 arriolac

@arriolac, I hope you can assign this issue to me, I will directly add the build workflow in Build.yaml to build the app-nia-catalog module.

However, there is a question about the test workflow of the app-nia-catalog module that needs to be answered, the app-nia-catalog module does not have the same functions as the app module and needs ui test. In the app module, the Navigation needs to do ui test. Do I need to add ui testing in the app-nia-catalog?

Great Thanks!

Rocksnake avatar Jun 05 '22 10:06 Rocksnake

@Rocksnake this issue is here for anyone to pick up, so feel free to take on :)

I think adding UI tests is out of scope for this issue, I was imaging this would purely be an improvement on the current workflow set up. Doing something like:

  1. Create a reusable workflow similar to https://github.com/android/compose-samples/blob/main/.github/workflows/build-sample.yml
  2. Update Build.yaml to use the reusable workflow
  3. Create a new workflow that to build/test app-nia-catalog

arriolac avatar Jun 06 '22 21:06 arriolac

I guess it make sense to wait for this PR https://github.com/android/nowinandroid/pull/130 to get merged

carmen-gh avatar Jul 05 '22 08:07 carmen-gh

@arriolac after looking at it more closely, compose-samples is monorepo with multiple apps while nowinandroid is single app repo with extra demo app.

Maybe this issue is outdated?

The current Build.yaml workflow is specific to building and testing the app module, however, there are more than 1 apps in the project.

Because current workflow builds and tests all modules, the only app specific steps are Upload build outputs & Upload build reports If we are interested in build outputs for app-nia-catalog it is easy to add, as for reusable workflow we can extract 2 steps mentioned above, yet it would be of questionable value since it's only lint reports and they are usually ignored by most devs unless configured to warningsAsErrors true

Would you please advise on how to proceed or close the issue?

YuraLaguta avatar Oct 25 '22 05:10 YuraLaguta

@YuraLaguta great that you are helping out with this. You can have the workflow accept two parameters (like uploadOutputs and uploadReports) which can default to true but overridden by app-nia-catalog to set as false so the behavior of outputs of the workflows won't change.

arriolac avatar Oct 25 '22 18:10 arriolac

@arriolac thanks for quick reply, I'm sorry for not being clear enough, the problem stated in the issue description does NOT exist.

The current Build.yaml workflow is specific to building and testing the app module

Current Build.yaml :

      ...
      - name: Build all build type and flavor permutations
        run: ./gradlew assemble --stacktrace

      - name: Run local tests
        run: ./gradlew testDemoDebug testProdDebug --stacktrace
      ...
      script: ./gradlew connectedProdDebugAndroidTest -x :benchmark:connectedProdBenchmarkAndroidTest --stacktrace

builds and tests all modules in the project.

YuraLaguta avatar Oct 26 '22 05:10 YuraLaguta

Thanks for clarifying. Yes, it makes sense to close this as the current workflow already accounts for both apps. I don't see much value in creating a separate workflow in that case.

arriolac avatar Oct 26 '22 16:10 arriolac