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

Update dependencies #1724

Closed
wants to merge 3 commits into from

Conversation

NyCodeGHG
Copy link
Contributor

Proposed Changes

This PR updates all dependencies to their latest versions.
A few dependencies such as ktor and logback had vulnerabilites before (also see #1720)
It also raises the build and runtime JDK requirements, as the Android Gradle Plugin requires running gradle with Java 17 and targeting Java 11. The Android APK Parser libraries used in maestro now also require at least Java 11 at runtime.

I also cherry-picked eec3de0 as the warning which existed with Gradle 7, is now a hard error in Gradle 8.

Testing

Ran the android advanced flow on an emulator.

Issues Fixed

@Fishbowler
Copy link
Contributor

Worth updating the GitHub Action too, so that you get the checks to ✅ ?

@lucaswiechmann
Copy link

Hey @NyCodeGHG
Could you please get the newest changes from main that were merged and update this PR ?

@NyCodeGHG
Copy link
Contributor Author

@lucaswiechmann done!

@lucaswiechmann
Copy link

Hey @NyCodeGHG thanks for that. Also, could you please check and fix the github actions?

Follow some details:

> Task :maestro-orchestra:compileTestKotlin
w: file:///home/runner/work/maestro/maestro/maestro-orchestra/src/test/java/maestro/orchestra/MaestroCommandSerializationTest.kt:57:13 'TapOnPointCommand' is deprecated. Use TapOnPointV2Command instead
w: file:///home/runner/work/maestro/maestro/maestro-orchestra/src/test/java/maestro/orchestra/MaestroCommandSerializationTest.kt:214:13 'AssertCommand' is deprecated. Use AssertConditionCommand instead
w: file:///home/runner/work/maestro/maestro/maestro-orchestra/src/test/java/maestro/orchestra/workspace/WorkspaceExecutionPlannerErrorsTest.kt:42:14 Parameter 'testCaseName' is never used

> Task :maestro-studio:server:compileJava NO-SOURCE
> Task :maestro-studio:server:classes
> Task :maestro-studio:server:jar
> Task :maestro-orchestra:compileTestJava NO-SOURCE
> Task :maestro-orchestra:testClasses
e: file:///home/runner/work/maestro/maestro/maestro-test/src/test/kotlin/maestro/test/IntegrationTest.kt:3059:33 Overload resolution ambiguity: 
public final fun isAtMost(other: Long?): Unit defined in com.google.common.truth.LongSubject
public final fun isAtMost(other: Int): Unit defined in com.google.common.truth.LongSubject

> Task :maestro-test:compileTestKotlin FAILED

> Task :maestro-orchestra:test

> Task :maestro-cli:compileKotlin
w: file:///home/runner/work/maestro/maestro/maestro-cli/src/main/java/maestro/cli/api/ApiClient.kt:352:60 Unchecked cast: Any? to Map<String, Any>
w: file:///home/runner/work/maestro/maestro/maestro-cli/src/main/java/maestro/cli/report/HtmlTestSuiteReporter.kt:120:15 'script(String? = ..., String? = ..., String = ...): Unit' is deprecated. This tag doesn't support content or requires unsafe (try unsafe {})
w: file:///home/runner/work/maestro/maestro/maestro-cli/src/main/java/maestro/cli/runner/TestRunner.kt:100:29 Variable 'cachedAppState' is never used

FAILURE: Build failed with an exception.

* What went wrong:

Deprecated Gradle features were used in this build, making it incompatible with Gradle 9.0.

You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.

@lucaswiechmann
Copy link

lucaswiechmann commented Jun 6, 2024

Hey @NyCodeGHG

This PR updates all dependencies to their latest versions.

Could you explain a little more the motivation to update all of those dependencies to their latest version?

A few dependencies such as ktor and logback had vulnerabilites before

Have you noticed other vulnerabilities other than the ones merged on this another PR ? If so, my suggestion is to update only the dependencies that has vulnerabilities.

More context: these changes could affect many people which are already using maestro today, besides that this brings major changes on how we manage our cloud internally, which we would need to test very careful before deciding to go ahead with this PR.

@NyCodeGHG
Copy link
Contributor Author

Could you explain a little more the motivation to update all of those dependencies to their latest version?

It's good practice to keep your dependencies up to date, so it's easier to update when a vulnerability gets public.
If your on the latest version it's usually just a patch release, but if you're (possibly multiple) major or minor versions behind, it could be a lot more work to update

Have you noticed other vulnerabilities other than the ones merged on this another #1720?

no

major changes on how we manage our cloud internally

i'm not sure i see the major change? the java version is probably the biggest update but other than that

@NyCodeGHG NyCodeGHG closed this Jul 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants