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

Embed proguard rule for generated provider class #341

Merged
merged 4 commits into from
Aug 25, 2023

Conversation

mataku
Copy link
Contributor

@mataku mataku commented Aug 11, 2023

Summary

Embed proguard rules for Codegen class so that minifyEnabled applications do not need additional rules.

Since the Codegen class extends the ShowkaseProvider class, additional rules are unnecessary for each application by providing rules for types that extend the ShowkaseProvider class.

consumerProguardRules is already specified in the project, so I added to it.

consumerProguardFiles "consumer-rules.pro"

refs. #199, #244

Context

Currently, we need to add extra proguard rules if we set minifyEnabled as true

@ShowkaseRootModule
class SampleShowkaseRootModule : ShowkaseRootModule

// Showkase generated class
public class SampleShowkaseRootModuleCodegen : ShowkaseProvider
# application proguard rules
-keep class **.SampleShowkaseRootModuleCodegen { }

Copy link
Collaborator

@vinaygaba vinaygaba left a comment

Choose a reason for hiding this comment

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

Thanks for the change!

@mataku
Copy link
Contributor Author

mataku commented Aug 11, 2023

@vinaygaba
There seems to be a problem with ui-testing workflow on GitHub Actions. I think macos-10.15 labelled runner is no longer available, so created a pull request to fix it: #342

Announcement: https://github.blog/changelog/2022-07-20-github-actions-the-macos-10-15-actions-runner-image-is-being-deprecated-and-will-be-removed-by-8-30-22/

Available Runners: https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners#supported-runners-and-hardware-resources

@mataku
Copy link
Contributor Author

mataku commented Aug 17, 2023

@vinaygaba
Thanks for merging #342! Is there anything else I should do?

@mataku
Copy link
Contributor Author

mataku commented Aug 18, 2023

The workflow execution seems to be unstable. The previous commit passed and the latest commit failed. I will look log details

@vinaygaba
Copy link
Collaborator

@mataku yeah these builds have been flaky and I have not had the time to investigate. Any help on this front will be greatly appreciated 🙏🏻

@mataku
Copy link
Contributor Author

mataku commented Aug 25, 2023

I think this pull request itself doesn't affect CI workflow's result because all projects in Showkase set minifyEnabled as false so generated apk doesn't apply R8 or proguard.

I investigated test results in my forked repo: https://github.com/mataku/Showkase/actions (WIP)

  • At first it is rare to encounter failing tests
    • You may see action run count itself is too small in above link, but I retried many times in each runs.
  • encountered com.airbnb.android.showkase_browser_testing.ShowcaseBrowserTest#customAnnotatedPrivateComposablesShouldCompileButNotShow but no logs printed about it
  • sample:connectedDebugAndroidTest often fails (but as noted above, in the first place, it is rare to encounter failing tests)
ShowcaseBrowserTest#customAnnotatedPrivateComposablesShouldCompileButNotShow results ss- 2023-08-25 14 09 08

@mataku
Copy link
Contributor Author

mataku commented Aug 25, 2023

@vinaygaba
How about treating instability in CI workflow as a new issue? If possible, I will create a new one.

For some time now, macOS specification on GitHub Actions has been out of date and the CI workflow has not been running, I don't understand the overall problem, but I think it was probably there originally as a potential issue.

In my investigation, the logs are printed unknown state, as if there is a system problem in the output or no logs at all in the first place, and the test will pass in many cases if it is retried. If tests related to changes in a pull request fail, the workflow will fail every time, so you can distinguish between the two.

@vinaygaba
Copy link
Collaborator

@mataku Yeah it's unrelated to your PR. I just would appreciate some help in figuring out what causes the flakiness. Additionally, even after removing the tests that were running on API 28, I continue to see them in the list above. I'll merge this PR in the meanwhile.

@vinaygaba vinaygaba merged commit fbce7d2 into airbnb:master Aug 25, 2023
@mataku
Copy link
Contributor Author

mataku commented Aug 27, 2023

@vinaygaba

Additionally, even after removing the tests that were running on API 28

I think you still have left the (deleted) API 28 workflow set as "Require status checks to pass before merging"? Please check the Branch protection rules in Showkase repository settings

sample screenshot screenshot 2023-08-27 19 37 34

vinaygaba added a commit that referenced this pull request Sep 19, 2023
* Revert proguard rule

* Update README with information about proguard
@vinaygaba
Copy link
Collaborator

@mataku I've reverted this PR. More explanation on why in the PR I updated it with - #354

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.

2 participants