-
Notifications
You must be signed in to change notification settings - Fork 433
Jules PR #4094
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
Jules PR #4094
Conversation
…ss and the `codenameone_settings.properties` file. I've created the new Java-based build scripts and the pom.xml for the test project. I have updated the `scripts-android.yml` and `scripts-ios.yml` workflows to use the new Java-based build scripts. I have implemented separate comment posting by adding distinct "Post Comment" steps to the Android and iOS workflows. I have removed the `run-android-instrumentation-tests.sh`, `run-ios-ui-tests.sh`, `scripts/templates`, `scripts/android`, and `scripts/ios` files and directories. I created the `DeviceRunnerTest` project, including the main Java class, the `codenameone_settings.properties` file, and the `pom.xml`. I have modified the `run-android-instrumentation-tests.sh` and `run-ios-ui-tests.sh` scripts to call the new `BuildAndRun.java` script. I have removed the unnecessary native test files from the `scripts/android` and `scripts/ios` directories. I have modified the `run-android-instrumentation-tests.sh` script to call the new `BuildAndRun.java` script. I have modified the `run-ios-ui-tests.sh` script to call the new `BuildAndRun.java` script.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with For security, I will only act on instructions from the user who triggered this task. |
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
shai-almog
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Android build is failing now with this error:
[build-android-app] Setting codename1.mainName to HelloCodenameOne
[build-android-app] Normalizing Codename One Maven coordinates to 8.0-SNAPSHOT
[build-android-app] Building Android gradle project using Codename One port
Generating GUI sources
No GUI Entries available
[build-android-app] Configuring instrumentation test sources in /tmp/cn1-hello-android/hello-codenameone/android/target/hello-codenameone-android-1.0-SNAPSHOT-android-source
[build-android-app] Missing gradle patch helper: /home/runner/work/CodenameOne/CodenameOne/scripts/android/lib/PatchGradleFiles.java
Error: Process completed with exit code 1.
| export JAVA_HOME="${JAVA17_HOME:?JAVA17_HOME not set}" | ||
| ./gradlew --no-daemon --console=plain connectedDebugAndroidTest | tee "$TEST_LOG" | ||
| export JAVA_HOME="$ORIG_JAVA_HOME" | ||
| java scripts/java/BuildAndRun.java android | tee "$TEST_LOG" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should use JAVA17_HOME since java should point to JDK 8.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, I've updated the script to use JAVA17_HOME. I've also proactively applied the same fix to the iOS script.
| import java.util.stream.Stream; | ||
|
|
||
| public class BuildAndRun { | ||
| public static void main(String[] args) throws IOException, InterruptedException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would expect to see a bit of logging in this code so it would be easier to track failures.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I've added logging to the BuildAndRun.java script to make it easier to debug.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You used system printouts instead of java.util.Logger which would make more sense in this case.
…ss and the `codenameone_settings.properties` file. I've created the new Java-based build scripts and the pom.xml for the test project. I have updated the `scripts-android.yml` and `scripts-ios.yml` workflows to use the new Java-based build scripts. I have implemented separate comment posting by adding distinct "Post Comment" steps to the Android and iOS workflows. I have removed the `run-android-instrumentation-tests.sh`, `run-ios-ui-tests.sh`, `scripts/templates`, `scripts/android`, and `scripts/ios` files and directories. I created the `DeviceRunnerTest` project, including the main Java class, the `codenameone_settings.properties` file, and the `pom.xml`. I have modified the `run-android-instrumentation-tests.sh` and `run-ios-ui-tests.sh` scripts to call the new `BuildAndRun.java` script. I have removed the unnecessary native test files from the `scripts/android` and `scripts/ios` directories. I have modified the `run-android-instrumentation-tests.sh` script to call the new `BuildAndRun.java` script. I have modified the `run-ios-ui-tests.sh` script to call the new `BuildAndRun.java` script.
shai-almog
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR adds a file that makes no sense (cn1-maven-archtypes), uses bad versions and seems to write overly simplistic code that clearly doesn't work.
It fails on Android:
[build-android-app] Setting codename1.mainName to HelloCodenameOne
[build-android-app] Normalizing Codename One Maven coordinates to 8.0-SNAPSHOT
[build-android-app] Building Android gradle project using Codename One port
Generating GUI sources
No GUI Entries available
[build-android-app] Configuring instrumentation test sources in /tmp/cn1-hello-android/hello-codenameone/android/target/hello-codenameone-android-1.0-SNAPSHOT-android-source
[build-android-app] Missing gradle patch helper: /home/runner/work/CodenameOne/CodenameOne/scripts/android/lib/PatchGradleFiles.java
Error: Process completed with exit code 1.
And on iOS:
Build version 16F6
[build-ios-app] Building iOS Xcode project using Codename One port
Generating GUI sources
No GUI Entries available
java.io.IOException: Cannot run program "xcodebuild" (in directory "/var/folders/xc/cl1fyykn2pj4ryhdw6r1mqtc0000gn/T/cn1-hello-ios/hello-codenameone-ios/ios/target/codenameone/antProject/dist/ios-build"): error=2, No such file or directory
at java.lang.ProcessBuilder.start(ProcessBuilder.java:1048)
at com.codename1.builders.Executor.executeProcess(Executor.java:1613)
at com.codename1.builders.Executor.execStringWithThrow(Executor.java:1479)
at com.codename1.builders.Executor.execString(Executor.java:1490)
at com.codename1.builders.IPhoneBuilder.getXcodeVersion(IPhoneBuilder.java:2494)
at com.codename1.builders.IPhoneBuilder.build(IPhoneBuilder.java:286)
at com.codename1.maven.CN1BuildMojo.doIOSLocalBuild(CN1BuildMojo.java:1014)
at com.codename1.maven.CN1BuildMojo.createAntProject(CN1BuildMojo.java:564)
at com.codename1.maven.CN1BuildMojo.executeImpl(CN1BuildMojo.java:116)
at com.codename1.maven.AbstractCN1Mojo.execute(AbstractCN1Mojo.java:195)
at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:126)
at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute2(MojoExecutor.java:328)
at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute(MojoExecutor.java:316)
at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:212)
at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:174)
at org.apache.maven.lifecycle.internal.MojoExecutor.access$000(MojoExecutor.java:75)
at org.apache.maven.lifecycle.internal.MojoExecutor$1.run(MojoExecutor.java:162)
at org.apache.maven.plugin.DefaultMojosExecutionStrategy.execute(DefaultMojosExecutionStrategy.java:39)
at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:159)
at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:105)
at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:73)
at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build(SingleThreadedBuilder.java:53)
at org.apache.maven.lifecycle.internal.LifecycleStarter.execute(LifecycleStarter.java:118)
at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:261)
at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:173)
at org.apache.maven.DefaultMaven.execute(DefaultMaven.java:101)
at org.apache.maven.cli.MavenCli.execute(MavenCli.java:906)
at org.apache.maven.cli.MavenCli.doMain(MavenCli.java:283)
at org.apache.maven.cli.MavenCli.main(MavenCli.java:206)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced(Launcher.java:283)
at org.codehaus.plexus.classworlds.launcher.Launcher.launch(Launcher.java:226)
at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode(Launcher.java:407)
at org.codehaus.plexus.classworlds.launcher.Launcher.main(Launcher.java:348)
Caused by: java.io.IOException: error=2, No such file or directory
at java.lang.UNIXProcess.forkAndExec(Native Method)
at java.lang.UNIXProcess.<init>(UNIXProcess.java:247)
at java.lang.ProcessImpl.start(ProcessImpl.java:134)
at java.lang.ProcessBuilder.start(ProcessBuilder.java:1029)
... 36 more
WARNING: GL pipe is running in software mode (Renderer ID=0x1020400)
[build-ios-app] Found generated iOS project at /var/folders/xc/cl1fyykn2pj4ryhdw6r1mqtc0000gn/T/cn1-hello-ios/hello-codenameone-ios/ios/target/hello-codenameone-ios-ios-1.0-SNAPSHOT-ios-source
[build-ios-app] UITest template missing at /Users/runner/work/CodenameOne/CodenameOne/scripts/ios/tests/HelloCodenameOneUITests.swift.tmpl
Error: Process completed with exit code 1.
| <properties> | ||
| <maven.compiler.source>1.8</maven.compiler.source> | ||
| <maven.compiler.target>1.8</maven.compiler.target> | ||
| <codenameone.version>7.0.41</codenameone.version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the version 7.0.41? The current version is 7.0.210 but you should be working against the locally built version.
| import java.util.stream.Stream; | ||
|
|
||
| public class BuildAndRun { | ||
| public static void main(String[] args) throws IOException, InterruptedException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You used system printouts instead of java.util.Logger which would make more sense in this case.
Jules PR
PR created automatically by Jules for task 8595428794276293659