Skip to content

Conversation

@wzieba
Copy link
Contributor

@wzieba wzieba commented Feb 20, 2024

Description

This PR adds tests to kotlin poet's code generators used in :libs:processors module.

Initially, I also wanted to introduce tests for code generation itself, performed in RemoteConfigProcessor, using kotlin-compile-testing. Unfortunately, we have a cycle dependency: :libs:processors module doesn't have access to org.wordpress.android.util.config.AppConfig class, but uses it during code generation in RemoteFeatureConfigCheckBuilder, making it impossible to test.

Additionally, this PR updates kotlin poet version to the newest stable and addresses breaking changes.

To Test:

Unit test should be just fine.


Regression Notes

  1. Potential unintended areas of impact

    • Code generation of classes:
      • RemoteFeatureConfigCheck
      • FeaturesInDevelopment
      • RemoteFeatureConfigDefaults
      • RemoteFieldConfigDefaults
  2. What I did to test those areas of impact (or what existing automated tests I relied on)

    • I wrote unit tests and smoke tested the app
  3. What automated tests I added (or what prevented me from doing so)

    • Unit tests

PR Submission Checklist:

  • I have completed the Regression Notes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

Testing Checklist:

  • WordPress.com sites and self-hosted Jetpack sites.
  • Portrait and landscape orientations.
  • Light and dark modes.
  • Fonts: Larger, smaller and bold text.
  • High contrast.
  • Talkback.
  • Languages with large words or with letters/accents not frequently used in English.
  • Right-to-left languages. (Even if translation isn’t complete, formatting should still respect the right-to-left layout)
  • Large and small screen sizes. (Tablet and smaller phones)
  • Multi-tasking: Split screen and Pop-up view. (Android 10 or higher)

@wzieba wzieba force-pushed the processor_unit_tests branch from 4775ece to cf2a4cc Compare February 20, 2024 11:54
@wzieba wzieba added the Testing label Feb 20, 2024
@wpmobilebot
Copy link
Contributor

Jetpack📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack Jetpack
FlavorJalapeno
Build TypeDebug
Versionpr20213-ec1baba
Commitec1baba
Direct Downloadjetpack-prototype-build-pr20213-ec1baba.apk
Note: Google Login is not supported on these builds.

@wpmobilebot
Copy link
Contributor

WordPress📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress WordPress
FlavorJalapeno
Build TypeDebug
Versionpr20213-ec1baba
Commitec1baba
Direct Downloadwordpress-prototype-build-pr20213-ec1baba.apk
Note: Google Login is not supported on these builds.

@codecov
Copy link

codecov bot commented Feb 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (db5bbc7) 40.18% compared to head (ec1baba) 40.31%.

Additional details and impacted files
@@            Coverage Diff             @@
##            trunk   #20213      +/-   ##
==========================================
+ Coverage   40.18%   40.31%   +0.13%     
==========================================
  Files        1469     1469              
  Lines       67672    67672              
  Branches    11211    11211              
==========================================
+ Hits        27193    27281      +88     
+ Misses      37985    37897      -88     
  Partials     2494     2494              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@wzieba wzieba marked this pull request as ready for review February 20, 2024 14:01
@wzieba wzieba added this to the 24.4 milestone Feb 20, 2024
@wzieba wzieba requested review from a team and antonis and removed request for a team February 20, 2024 14:01
Copy link
Contributor

@antonis antonis left a comment

Choose a reason for hiding this comment

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

Thank you for your work on this @wzieba 🙇
The code changes look consistent to me and the added tests make sense. I also didn't notice any side effects in the code generation 🎉

@antonis antonis merged commit 40ece18 into trunk Feb 20, 2024
@antonis antonis deleted the processor_unit_tests branch February 20, 2024 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants