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

Upgrade to AGP 8 & Kotlin 1.9.10 #324

Merged
merged 18 commits into from
Jan 4, 2024
Merged

Upgrade to AGP 8 & Kotlin 1.9.10 #324

merged 18 commits into from
Jan 4, 2024

Conversation

mattkranzler5
Copy link
Contributor

@mattkranzler5 mattkranzler5 commented Dec 14, 2023

  • Upgraded to AGP 8.2.0
  • Upgraded to Kotlin 1.9.10
  • Upgraded to Java 11
  • Consolidated common config in root build.gradle.kts

@Jawnnypoo
Copy link
Member

Take a look at #300 for some of the issues faced there too.

@mattkranzler5
Copy link
Contributor Author

Take a look at #300 for some of the issues faced there too.

Yep, I've referenced that a few times. Only outstanding issue is for some reason mockito cannot mock a formula class for a test. Trying to resolve but haven't been able to figure out a workaround yet.

Comment on lines 19 to 21
class MyFormula : Formula<Unit, Unit, Unit>() {
override fun initialState(input: Unit) = Unit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Laimiux this test was failing due to an issue mocking MyFormula as a StatelessFormula:

Caused by: java.lang.reflect.MalformedParameterizedTypeException: Mismatch of count of formal and actual type arguments in constructor of com.instacart.formula.StatelessFormula$implementation$1: 0 formal argument(s) 2 actual argument(s)
	at java.base/sun.reflect.generics.reflectiveObjects.ParameterizedTypeImpl.validateConstructorArguments(Unknown Source)
	at java.base/sun.reflect.generics.reflectiveObjects.ParameterizedTypeImpl.<init>(Unknown Source)
	at java.base/sun.reflect.generics.reflectiveObjects.ParameterizedTypeImpl.make(Unknown Source)
	at java.base/sun.reflect.generics.factory.CoreReflectionFactory.makeParameterizedType(Unknown Source)
	at java.base/sun.reflect.generics.visitor.Reifier.visitClassTypeSignature(Unknown Source)
	at java.base/sun.reflect.generics.tree.ClassTypeSignature.accept(Unknown Source)
	at java.base/sun.reflect.generics.repository.FieldRepository.computeGenericType(Unknown Source)
	at java.base/sun.reflect.generics.repository.FieldRepository.getGenericType(Unknown Source)
	at java.base/java.lang.reflect.Field.getGenericType(Unknown Source)
	at net.bytebuddy.description.type.TypeDescription$Generic$LazyProjection$ForLoadedFieldType.resolve(TypeDescription.java:6685)
	at net.bytebuddy.description.type.TypeDescription$Generic$LazyProjection.accept(TypeDescription.java:6297)
	at net.bytebuddy.description.field.FieldDescription$AbstractBase.asToken(FieldDescription.java:200)
	at net.bytebuddy.description.field.FieldDescription$AbstractBase.asToken(FieldDescription.java:123)
	at net.bytebuddy.description.field.FieldList$AbstractBase.asTokenList(FieldList.java:64)
	at net.bytebuddy.dynamic.scaffold.InstrumentedType$Factory$Default$1.represent(InstrumentedType.java:436)
	at net.bytebuddy.ByteBuddy.redefine(ByteBuddy.java:886)
	at net.bytebuddy.ByteBuddy.redefine(ByteBuddy.java:861)
	at org.mockito.internal.creation.bytebuddy.InlineBytecodeGenerator.transform(InlineBytecodeGenerator.java:390)
	at java.instrument/java.lang.instrument.ClassFileTransformer.transform(Unknown Source)
	at java.instrument/sun.instrument.TransformerManager.transform(Unknown Source)
	at java.instrument/sun.instrument.InstrumentationImpl.transform(Unknown Source)
	at java.instrument/sun.instrument.InstrumentationImpl.retransformClasses0(Native Method)
	at java.instrument/sun.instrument.InstrumentationImpl.retransformClasses(Unknown Source)
	at org.mockito.internal.creation.bytebuddy.InlineBytecodeGenerator.triggerRetransformation(InlineBytecodeGenerator.java:281)
	... 58 more

I changed MyFormula to extend Formula as a workaround which seems ok based on what the actual test is trying to test.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I suspect this test was added to ensure that mockFormula would continue to work in our codebase.

@carrotkite
Copy link

carrotkite commented Dec 14, 2023

JaCoCo Code Coverage 78.89% ✅

Class Covered Meta Status
com/instacart/formula/StatelessFormula 100% 0%

Generated by 🚫 Danger

@mattkranzler5 mattkranzler5 changed the title Upgrade to AGP 8 & Kotlin 1.9.20 Upgrade to AGP 8 & Kotlin 1.9.10 Dec 18, 2023
uses: actions/setup-java@v2
with:
distribution: 'zulu'
java-version: 17
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if this should be java 18?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call, updated.

@Laimiux Laimiux merged commit 50ce472 into master Jan 4, 2024
5 checks passed
@Laimiux Laimiux deleted the mk/agp-kotlin-upgrade branch January 4, 2024 14:43
@Jawnnypoo Jawnnypoo mentioned this pull request Jan 4, 2024
Laimiux added a commit that referenced this pull request Jan 26, 2024
* Upgrade to AGP 8 & Kotlin 1.9.20

* Use java 17

* Update jacoco version

* Fix namespace in stopwatch

* Update compose version & remove namespace from manifest files

* Fix compilation errors, update mockito

* Update compile sdk to 33

* Update compile sdk to 34

* Update to latest versions of mockito

* Fix for mockito test

* Fix test compilation error

* Fix test compilation error

* Revert change with R namespace change, update robolectric

* Use kotlin 1.9.10 to match IC App

* Revert to compose 1.5.3 which supports kotlin 1.9.10

* Use jdk 18

* Fix mockito.

* Use java 18.

---------

Co-authored-by: Laimonas Turauskas <[email protected]>
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.

4 participants