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

Adds back the JUnit testing system #4979

Merged
merged 48 commits into from
Mar 8, 2023
Merged

Adds back the JUnit testing system #4979

merged 48 commits into from
Mar 8, 2023

Conversation

TheLimeGlass
Copy link
Collaborator

@TheLimeGlass TheLimeGlass commented Jul 27, 2022

Description

Adds back the JUnit testing system.

  • Adds JUnit tests under src/test/java again.
  • All tests under src/test/java are ran with the test runner server using JUnit.
  • Renamed ch.njol.skript.tests to ch.njol.skript.test so that static loadClasses can read ch.njol.skript.test.tests without other classes interfering.
  • The old JUnit testing used to run each time the jar was built, this was annoying but not an issue. This pull request keeps the runtime tests separate from the gradlew clean build task.
  • Cleaned up the formatting of the test results to be much better. (Failed tests could have been displayed as success)
  • Gradle tasks are JUnitJava17, JUnitJava8 and JUnitQuick (Capitalization doesn't matter)
  • Added a folder in the test scripts named "junit" and a readme explaining how to do JUnit scripts combined with Java classes.
  • Reduced the test failure timeout to 5 minutes.
  • All exceptions that happen in the JUnit tests prints as Skript.exception formatted.
  • Added expression [the] [current[ly [running]]] junit test [name] for test scripts to be able to figure out what JUnit test is being ran currently. This opens the possibility of anything to be done with Java to Skript for testing.
  • Moves the /test_runners folder into /build/test_runners so that you can use gradlew clean or no clean if you don't want to clear the build cache. (Pickle and I wanted this in a conversation)
  • Adds an objective completeness system. A script defines string objectives that the JUnit needs to complete in conjunction with any runtime things the test is doing. New syntax at EffObjectives in test runner package.

Notes:

  • This is superior to Implementation of Java tests #4670 because it actually uses the JUnit library and runs along side the test runner server without needing a script .sk existing, just a typical JUnit class.
  • JUnit 4 is the only version that has the JUnitCore runner that allows runtime unit checking, so we have to use JUnit 4, which is fine, we just need standard JUnit functionality.
  • Adds EasyMock support Usage of EasyMock in Skript tests (Create fake objects like Players) #5033
  • The GitHub actions are split into two checks for speed completion.

Related Issues: #4973

@TheLimeGlass TheLimeGlass marked this pull request as draft July 27, 2022 08:53
@TheLimeGlass TheLimeGlass added feature Pull request adding a new feature. unit testing For issues/PRs related to the Skript unit testing system. labels Jul 27, 2022
@TheLimeGlass TheLimeGlass changed the title Add a proper JUnit testing system Adds back the JUnit testing system Jul 27, 2022
@TheLimeGlass

This comment was marked as outdated.

@TheLimeGlass TheLimeGlass marked this pull request as ready for review August 17, 2022 07:47
@TheLimeGlass TheLimeGlass marked this pull request as ready for review December 28, 2022 12:07
@TheLimeGlass TheLimeGlass requested a review from kiip1 January 27, 2023 07:36
Copy link
Contributor

@kiip1 kiip1 left a comment

Choose a reason for hiding this comment

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

All good! When #5299 is merged you can remove some duplicate code as TP said.

Copy link
Member

@Moderocky Moderocky 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 :)

Copy link
Member

@AyhamAl-Ali AyhamAl-Ali left a comment

Choose a reason for hiding this comment

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

Well done 🚀

@TheLimeGlass TheLimeGlass requested review from TPGamesNL and removed request for TPGamesNL February 16, 2023 12:59
Copy link
Member

@AyhamAl-Ali AyhamAl-Ali left a comment

Choose a reason for hiding this comment

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

Looking good 👏

build.gradle Show resolved Hide resolved
build.gradle Show resolved Hide resolved
Copy link
Member

@APickledWalrus APickledWalrus 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. Once TP's comments are addressed and his approval is obtained, this should be good to go.

@TheLimeGlass TheLimeGlass removed the request for review from TPGamesNL March 8, 2023 08:41
@TheLimeGlass TheLimeGlass dismissed TPGamesNL’s stale review March 8, 2023 08:41

Applied all changes

@TheLimeGlass TheLimeGlass merged commit 2334e0a into master Mar 8, 2023
@TheLimeGlass TheLimeGlass deleted the feature/junit branch March 8, 2023 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.7 Targeting a 2.7.X version release feature Pull request adding a new feature. unit testing For issues/PRs related to the Skript unit testing system.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants