Skip to content

Conversation

@zhenglong1603
Copy link

@zhenglong1603 zhenglong1603 commented Aug 4, 2025

Update GitHub Actions workflow to latest versions

This PR updates the CI workflow with the following changes:

Upgrades the Gradle wrapper validation action from gradle/wrapper-validation-action@v1 to gradle/actions/wrapper-validation@v3.

Updates the Java setup step from actions/setup-java@v1 to actions/setup-java@v4, using JDK 17 with the jdk+fx package.

Installs dos2unix on both Ubuntu (apt-get) and macOS (brew) runners to normalize line endings and prevent issues with shell scripts across platforms.

These updates ensure the workflow uses the latest stable GitHub Actions and Java versions, improves cross-platform compatibility, and enhances security and maintainability.

@zhenglong1603 zhenglong1603 changed the title Update GitHub Actions workflow [#80] Update GitHub Actions workflow Aug 4, 2025
@zhenglong1603 zhenglong1603 changed the title [#80] Update GitHub Actions workflow [#66] Update GitHub Actions workflow Aug 4, 2025
@damithc
Copy link
Collaborator

damithc commented Aug 6, 2025

@zhenglong1603 Did you do any testing to see if everything works?

@zhenglong1603
Copy link
Author

zhenglong1603 commented Aug 6, 2025

@damithc,
I ran a clean build and executed Duke.main(). I've attached the relevant screenshots. Please let me know if there are any additional tests I might have missed.
image
image

@damithc
Copy link
Collaborator

damithc commented Aug 6, 2025

@zhenglong1603 You can also try the following:

Copy over your clone to a new folder.
Merge this branch to the master branch.
Add some code (you can copy over your java code from the ip, including Junit code)
Confirm the code works locally.
Create an empty repo on GitHub and push this code. Enable GitHub actions.
Confrim the GitHub actions workflow completes without any issue.

@zhenglong1603
Copy link
Author

zhenglong1603 commented Aug 7, 2025

Hi Prof @damithc,
I followed the steps you suggested and copied the Java code over from my IP. It worked locally, as shown in the attached screenshot. I also created a JAR file and tested it successfully.

However, after pushing the code to the GitHub repository, the GitHub Actions workflow failed. I think it’s because JavaFX isn’t accounted in the workflow configuration. Do we need to tweak the checks to account for JavaFX in the workflow?

image

@damithc
Copy link
Collaborator

damithc commented Aug 7, 2025

@zhenglong1603 can you share the link to the failed PR workflow?
Also, compare with AB3 workflow file to see which part is missing in this workflow file?

@zhenglong1603
Copy link
Author

zhenglong1603 commented Aug 7, 2025

Hi Prof @damithc,

The link would be https://github.com/zhenglong1603/duke-testing/actions

I compared with the AB3 workflow. I believe its because in AB3 workflow file, it just runs ./run-checks.sh which I believe runs these following checks in the screenshot which just check the files whereas in Duke, it runs ./runtest.sh which I believe launches the main application. This likely triggers the GUI (e.g., by calling Application.launch(...)), causing the failure in headless environments like the GitHub Actions.

I suspect this wasn’t an issue earlier because Duke doesnt include any JavaFX dependencies so the tests will run well without triggering any GUI issue.

Given that Duke doesn't have a GUI by default whereas AB3 does, would seek your opinion on this.
image
image

image

@damithc
Copy link
Collaborator

damithc commented Aug 7, 2025

@zhenglong1603 Thanks for checking. Can you try without the GUI code (e.g., an earlier version of the iP)?

@zhenglong1603
Copy link
Author

zhenglong1603 commented Aug 7, 2025

Hi Prof @damithc,

I've added an earlier version of the IP code as you suggested. Below are the screenshots of it working both locally and after generating the JAR file. The GitHub Actions workflow also ran successfully.

I did notice a message saying "dos2unix not found". I tested adding dos2unix installation to the workflow, and it resolved the warning. However, since everything worked fine even without it, I'm unsure whether it's necessary to include it since I think its a minor issue. Might need your opinion on it.

image image image image

After adding the installation of dos2unix in workflow,
image
image

@damithc
Copy link
Collaborator

damithc commented Aug 7, 2025

I did notice a message saying "dos2unix not found". I tested adding dos2unix installation to the workflow, and it resolved the warning. However, since everything worked fine even without it, I'm unsure whether it's necessary to include it since I think its a minor issue. Might need your opinion on it.

Good work, @zhenglong1603
yes, go ahead and add dos2unix to the workflow.

@zhenglong1603
Copy link
Author

@damithc
Noted, Prof. I’ve made a new commit with the necessary changes. Please let me know if there are any further issues. Thank you!

@damithc
Copy link
Collaborator

damithc commented Aug 7, 2025

Thanks for this fix, @zhenglong1603 💯
I've merged it after tidying up the commits a bit.

@damithc damithc closed this Aug 7, 2025
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.

2 participants