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

test: use Gradle wrapper for Java track #1126

Merged

Conversation

sanderploegsma
Copy link
Contributor

@sanderploegsma sanderploegsma commented Jan 21, 2024

This updates the exercism test command to use the Gradle wrapper for exercises on the Java track.

Dependencies:

@sanderploegsma sanderploegsma marked this pull request as ready for review January 22, 2024 08:36
Copy link

@arlm arlm left a comment

Choose a reason for hiding this comment

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

Looks good to me

@ErikSchierboom
Copy link
Member

Note that on Linux, this requires student to set the executable bit before it can work. @sanderploegsma would you still want this to be merged?

@sanderploegsma
Copy link
Contributor Author

That's right, thanks for reminding me! To be honest I'm not sure what the right approach is here.

Right now the installation instructions on the Java track do not include the installation of Gradle and the working locally instructions point to using the Gradle wrapper. However, this already requires students on unix-based systems to set the executable bit before they can run tests locally, unless they happen to use a local installation of Gradle instead.

The ideal solution here is still to retain file permissions when downloading exercises using the CLI. I know there is an open discussion about this in #903, perhaps it's a good idea to pick that back up? It's already mentioned there, but in the Java/Kotlin world it's considered a best-practice to use the Gradle/Maven wrappers so I really think this approach should work out-of-the-box on Exercism as well.

@sanderploegsma
Copy link
Contributor Author

Also I'd like to note that these changes at least align the Java and Kotlin tracks again. Plus, when a student is starting out with working locally and following the instructions they will:

  1. Install the Exercism CLI
  2. Install Java
  3. Download an exercise using the CLI
  4. Invoke exercism test
  5. Get an error about the gradle command not being found.

With these changes step 5 will change to the ./gradlew command not being executable, which is IMO easier to address. Plus, Windows users shouldn't run into issues at all.

@ErikSchierboom
Copy link
Member

I know there is an open discussion about this in #903, perhaps it's a good idea to pick that back up?

Yeah let's do that. Maybe for now we'll merge this?

@sanderploegsma
Copy link
Contributor Author

Yeah let's do that 👍

@uzilan
Copy link

uzilan commented May 7, 2024

I'd like to suggest that you add instructions in Testing on the Java track that explain how to chmod the .gradlew file in case people are unsure why they get the error message when running the tests

@ErikSchierboom ErikSchierboom merged commit db9c0e8 into exercism:main May 7, 2024
7 checks passed
@endoarato
Copy link

One issue that I found for Windows specifically is when I use Powershell 7 or nushell, 'gradlew.bat test' does not work. In both, I need to use "./gradlew.bat test'. I think it would be good to make exercism be aware of the shell that it is currently run in, since the Windows command seems to assume 'cmd'.

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.

5 participants