Test Clean Up #1096
Test Clean Up #1096
Conversation
These are just some simple test fixes, mostly caught by IntelliJ's static analysis and SonarQube's static analysis. Below are the changes: 1. Corrected order of parameters for `assertEquals`. `expected` should be the first parameter with `actual` being the second parameter. 2. Remove unnecessary `throws IOException` in `McVersionLookupTest`. As a note, all parameters and the return for `visitFile` should be marked NotNull (or NonNull if you migrate to JSpecify). An interface may state the throw in the function signature but in Java, if overriding a method, you cannot declare new or broader checked exceptions but you can declare fewer, and if your implementation doesn't throw, then you shouldn't include it in the override's signature (which is why IDEs like IntelliJ will tell you to remove it). 3. Statement lambdas were replaced with expression lambdas where appropriate. 4. Some tests used `Assertions.assertEquals` while others just directly imported `assertEquals`. I changed all instances of `Assertions.assertEquals` to `assertEquals` to keep things consistent. 5. In `V1ModJsonParsingTests`, the field `private static Path testLocation` was converted to a local variable. It was never used outside of `setupPaths`. 6. An `assertTrue` check was correctly changed to `assertInstanceOf` in `V1ModJsonParsingTests`. 7. Instances of explicit type arguments that were redundant were simplified to `<>`. This doesn't change any functionality, all tests run the same. This only corrects some semantics (like incorrect `assertEquals` order) and cleans up code.
|
You have previously been perm-banned from FabricMC after a series of inappropriate behavior. We do not accept this PR unless you file an appeal first. To appeal, please contact any one of the current moderators (apple502j, shnupbups, modmuss50) on Discord. |
|
Hi @apple502j! I actually tried to contact all three of you a few years ago to submit an appeal and clear the air, but I wasn't able to send any of you a friend request since we didn't share any servers. There also doesn't seem to be an appeal server or form available, so I wasn't sure how to reach out for quite some time. I'm now able to message shnupbups on Discord because we now share a server. I've gone ahead and sent him a message. I look forward to resolving this and appreciate your time. |
|
@apple502j I've appealed and have been unbanned. Could you reopen this PR? |
3 participants
These are just some simple test fixes, mostly caught by IntelliJ's static analysis and SonarQube's static analysis. Below are the changes:
assertEquals.expectedshould be the first parameter withactualbeing the second parameter.throws IOExceptioninMcVersionLookupTest. As a note, all parameters and the return forvisitFileshould be marked NotNull (or NonNull if you migrate to JSpecify). An interface may state the throw in the function signature but in Java, if overriding a method, you cannot declare new or broader checked exceptions but you can declare fewer, and if your implementation doesn't throw, then you shouldn't include it in the override's signature (which is why IDEs like IntelliJ will tell you to remove it).Assertions.assertEqualswhile others just directly importedassertEquals. I changed all instances ofAssertions.assertEqualstoassertEqualsto keep things consistent.V1ModJsonParsingTests, the fieldprivate static Path testLocationwas converted to a local variable. It was never used outside ofsetupPaths.assertTruecheck was correctly changed toassertInstanceOfinV1ModJsonParsingTests.<>.This doesn't change any functionality, all tests run the same. This only corrects some semantics (like incorrect
assertEqualsorder) and cleans up code.