-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
test(android, lint): enable java lint check in CI #5208
Conversation
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/invertase/react-native-firebase/48MPQYVqTfDPrhdnHcNUvAPjaiNe |
Codecov Report
@@ Coverage Diff @@
## master #5208 +/- ##
===========================================
- Coverage 88.87% 36.59% -52.27%
===========================================
Files 109 51 -58
Lines 3744 1517 -2227
Branches 360 360
===========================================
- Hits 3327 555 -2772
- Misses 370 732 +362
- Partials 47 230 +183 |
API21 emulator is not easily attainable on github CI runners, probably have to do a local update of SDK tools to get it's definition, and may need to install API21 API as well (unsure) but I did not have success with Detox getting it to run unfortunately, so this needs a lot more investigation Alternative (or additive) solution for this is to enable a lintRelease check in CI that checks for react-native-firebase/packages/app/android/build.gradle Lines 58 to 61 in 1510502
Instead I think we should have a common lint.gradle and all the modules should refer to it, with settings for NewApi to fail - when I toggle abortOnError to true in the file referenced above, this is correctly flagged as a build failure. |
Java lint was not being checked anywhere, and the NewApi check in particular would have stopped a crash. Add full lint configuration tuned for current status Cannot enable it on the modules or our modules may fail library-consumer's builds, but we may configure it on the sub-modules from our test app then check that Tested and working and would have correctly failed the NewApi violation shown in #5206
d68b9d4
to
870705b
Compare
The most important part - and the only part I changed - was to add lintDebug to the android e2e run, and I verified the log it was perfect. Merging |
Description
If we say we support minSdkVersion 16 we should check it somehow
It turns out that getting a non-API30 emulator is somewhat difficult in CI, but...
Java lint was not being checked anywhere, and the NewApi check in particular
would have stopped a crash. So add full lint configuration tuned for current status
Cannot enable it on the modules or our modules may fail library-consumer's builds,
but we may configure it on the sub-modules from our test app then check that
Tested and working and would have correctly failed the NewApi violation shown in #5206
Related issues
Related #5206 - Android 7 crash from use of Java8 features in #4981
Release Summary
Conventional commit message
Checklist
Android
iOS
e2e
tests added or updated inpackages/\*\*/e2e
jest
tests added or updated inpackages/\*\*/__tests__
Test Plan
The change is literally a test :-)
Think
react-native-firebase
is great? Please consider supporting the project with any of the below:React Native Firebase
andInvertase
on Twitter