-
Notifications
You must be signed in to change notification settings - Fork 77
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
Set revision when building jar with gradle #17
Conversation
@gausie Feel free to assign this PR to me if or when you want me to take a look |
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.
Looks good, modulo some nits that I don't feel strongly about. Thanks!
|
||
Manifest manifest = new Manifest( resources.openStream() ); | ||
String buildRevision = manifest.getMainAttributes().getValue( "Build-Revision" ); | ||
if ( buildRevision != null && StringUtilities.isNumeric( buildRevision ) ) |
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.
Optional: As noted in the previous PR, I think wrapping Integer.parseInt in a try/catch would be sufficient here and not be subject to the nonstandard parsing rules present in StringUtilities.isNumeric / parseInt.
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.
(And as we both noted, it's not a meaningful change of behavior, in most cases. It just accepts and caches a slightly broader set of inputs.)
@heeheehee-kolmafia I'm having some issues whereby the revision always comes out at 0. Trying to work it out... |
@gausie What situations are you seeing the revision as 0? I built a fresh jar, which was mostly named appropriately (marked as M even though my branch is up-to-date, although I do have some untracked files in the repo root). And, both the get_revision() ASH function and the version string ("KoLmafia r25900" or so) looked good. The ant build on the other hand had revision = 0, which is unsurprising given that it doesn't set the manifest appropriately. |
build.gradle
Outdated
@@ -121,7 +121,7 @@ jar { | |||
duplicatesStrategy = 'exclude' | |||
destinationDirectory = file('dist/') | |||
archiveBaseName = "KoLmafia" | |||
classifier = "${versioning.info.dirty ? 'Modified' : ''}" | |||
classifier = "${versioning.info.dirty ? 'M' : ''}" |
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.
(Note this is subtly different from what we have today with ant daily
, namely we'll now have "KoLmafia-r12345-M.jar" instead of "KoLmafia-r12345M.jar". I don't think this is a meaningful difference.)
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.
Yeah me and @BadHorseMonkey noticed this and thought that it might be alright given it allows you to split by - and grab [1]
Some other misc comments, that I don't think are blocking:
|
I had this issue when I built a jar though @BadHorseMonkey didn't have the issue ejther. Me and @BadHorseMonkey found ourselves unable to change flatlaf themes, can you test that? |
I too can't change flatlaf themes. I don't see them bundled in the jar. Perhaps all the runtimeOnly deps should actually be implementation deps. Simple enough fix. |
I think I'd like to leave this to @Rinn when he's available! |
I get the feeling that my revision not being found in the jar could be a Linux compatibility issue? |
Could be, but we should be able to find/fix tha
Before we do that, I have a two line fix that's working in my repo. `plugins { } Then refresh your gradle tasks and in the new shadow subfolder, chose shadowJar. There may be more cleanup, and it uses the default shadowJar directories (build/libs for the jar). Also, I updated my gradle to the latest code that has the platform-agnostic files, and now I get revision 0 as well... |
By Linux compatibility issue, do you mean because you're using Linux, or that it only works on Linux? Because it works for me, on Linux. I've been building and running off of this branch's head with no issues. Can you unzip the resulting jar and see what's actually being written into your META-INF/MANIFEST.MF? Do you need to set mainClassName outside of application { mainClass }? Hm. |
(Also, with changing runtimeOnly -> implementation, I was able to change flatlaf themes. Jar got a bit bigger, but that's expected given that we were omitting the flatlaf files previously...) |
I'm pretty sure this is because shadowJar extends jar, but it's a separate plugin task. There might be other ways to do it, but I tried adding the shadow plugin and then it complained. I added the mainClassName and it stopped complaining. It's how my other side project works, so I was pretty sure it would work here, too... |
@BadHorseMonkey could you add commits to that effect to this branch? You should have permission to do so |
With shadowJar I too get revision == 0, but that's because shadowJar doesn't depend on the version-setting logic (only jar does). shadowJar.dependsOn gitRevList fixes that for me. |
I'll see if I can. |
Merging this, @Rinn it would be great if you could look over and throw in a supplementary PR if you deem it necessary. This is working great though, we just need to run Also would be good to see if we can slim down the |
* Set revision when building jar with gradle (#17) * Add revision to jar manifest and load in code * Override revision in tests * Remove RELEASED * Enable releases * move to StaticEntity, add build info * Fix tests for revision numbers * Change product name to CONSTANT_CASE * Capitalize getBuildInfo properly * We only have one MANIFEST file * Use r0 instead of rUnknown, make r0 pass all `since` checks * Remove another manifest loop * Change Modified classifier back to M * Use system-agnostic File constructor on gradle * Added shadow plugin * Fix version numbers - we have to use shadowJar * Final tidy-up * Do not run the PR tests twice Co-authored-by: Rinn <Kirchoff.Joseph.P@gmail.com> Co-authored-by: BadHorseMonkey <58645293+BadHorseMonkey@users.noreply.github.com> * Create jenkinsfile (#16) * Create jenkinsfile Adding the initial version of jenkinsfile for ci.kolmafia.us * Update jenkinsfile change build target to jar to update revision numbers * Update jenkinsfile Update target to shadowJar * feat: add portable Spacegate (open) (kolmafia#19) * dev: use 3-clause BSD license (kolmafia#18) * Publish test report in coverage, fix revision on release, fix NullPointerExecption (kolmafia#26) * Build shadowJar instead of jar for releases (kolmafia#31) * Remove individual licenses in favour of root license (kolmafia#21) * license which evaded the deletion (kolmafia#32) * Language Server - parsetree abstracts (kolmafia#30) ( from https://kolmafia.us/threads/ash-language-server-features.26098/post-163993 ) A few appetizers while we wait for the rest: * The classes CompositeType, CompositeValue and AggregateValue should be made abstract. * The method CompositeType.getDataType( Object ) can be made abstract. Rather than having Type.toString() and RecordType.toString() both be return this.name, just make Symbol.toString() { return this.name; } (this also incidentally adds the method to Variable and Function, which helps in the debugger) * Language Server - parseType records (kolmafia#29) ( from https://kolmafia.us/threads/ash-language-server-features.26098/post-163762 ) This changes the logic of parseType to attempt to parse records even when not allowed (again, with the continuous parsing, we need to try anyway). This is possible because the method parseRecord starts by looking for the token "record", immediately returning null if it doesn't find it. "record" is a reserved word exclusively used to declare new records, so nothing is hindered by checking for them right away. * Language Server - parseFunction vararg (kolmafia#28) ( from https://kolmafia.us/threads/ash-language-server-features.26098/post-163765 ) This modifies the logic of parseFunction a bit so that the vararg error checks happen after the variable name was read. This will allow us to have the full variable + type's location, rather than only the type, once those are implemented. Additionally, this allows the "Only one vararg parameter is allowed" error message to be reached. * Language Server - parseAggregateLiteral arrays (kolmafia#22) (from https://kolmafia.us/threads/ash-language-server-features.26098/post-163764 ) Alters parseAggregateLiteral a little so that we try to read key-value pairs (components of a map literal) even once we identified that the user is supplying an array literal (which doesn't expect keys). * Language Server - parseVariable initializations (kolmafia#27) ( from https://kolmafia.us/threads/ash-language-server-features.26098/post-163766 ) A bit more complex than the last few; this patch modifies parseVariable so that the signal indicating that we're parsing parameters is a boolean, rather than scope being null, and waits until the end to throw the error. This, again, will allow us to still keep trying to parse a parameter initialization, and will give us a full location once we throw the error. Also shuffles the parsing logic a little, for efficiency. * not supposed to touch that one either..? Why'd the test work anyway, then..? (context: kolmafia@d353d73) * way to pass the variable name (and location, in the future!) * Updated test to initialize TurnCounters prior to testing them. Not all systems need this, apparently. (kolmafia#33) * to_int() and the .id proxy record value can now extract the snarfblat from a $location. Non-adventure.php-locations will return -1 (kolmafia#34) * Update jenkinsfile (kolmafia#25) * Update jenkinsfile Update jenkinsfile to use --no-daemon and change the test build targets. * Update jenkinsfile Added a larger shallow depth * Update jenkinsfile * Update jenkinsfile Update Java version as 1.8 was causing issues on Jenkins instance. Now building and testing against Java 16. * Move over sourceforge references and remove unused donation section (kolmafia#35) * Move over sourceforge references and remove unused donation section * Include buffbot.xsl file in repo * Remove VariableReference(String, BasicScope) (kolmafia#39) * turn (Type) == (Type) into Type.equals( Type ) (kolmafia#36) * Prune old jars when building with gradle (kolmafia#40) * Prune old jars in when building with gradle * Force gradlew to always use unix line endings * Add line ending to end of file * Delete -M if not dirty, etc * Change some ints to longs in BasementRequest (kolmafia#42) * feat: try multiple faxbots if the first fails in the CLI (kolmafia#37) * feat: try multiple faxbots if the first fails in the CLI * chore: fmt * fix: also try another bot if many matches for command * Security lint (kolmafia#45) Addressed instances of Implicit narrowing conversion in compound assignment flagged by security checking. Trivial since the issue was just a local int that should have been changed when MP, HP, etc. went to longs. Remaining instances seem to require changing global structures or method signatures. Keeping this small to validate that I know how to "commit changes with git". * Update DiscoCombatHelper.java * Implicit naaarrowing Address instances of "Implicit narrowing conversion in compound assignment" * Language Server - Command class (kolmafia#20) * manually re-add to ef2d685 5672b9a a3d01c9 6a57eae * remove license from Command.java * Use Java 9, add spotless w/ google java style. (kolmafia#47) This is a proof-of-concept, and does not actually apply any formatting changes yet. I'd prefer to make that formatting change in a standalone commit. This also removes some comments in the middle of imports in KoLConstants.java because `./gradlew spotlessCheck` was yelling at me, because Google Java Format (gojf) doesn't like that. To reformat the code, you'll run `./gradlew spotlessApply`. This adds some flags so gojf continues to work for newer Java versions (16+). But for that, requires that the Java compiler be new enough to support exports, aka at least Java 9, which coincidentally is the highest version supported by OpenClover. We really should switch away from OpenClover to something like JaCoCo. (Coming soon!) * Implicit Narrowing part 2 (kolmafia#46) Address additional implicit narrowing warnings * Migrate from OpenClover to JaCoCo for coverage (kolmafia#48) * Use JaCoCo for coverage instead of Clover. They have support for Java > 9... * Remove Clover coverage comment. This branch, if merged, will mean that we're no longer using Clover, so it'll be moot. * Update jenkinsfile, too. I think this should work, but I could be wrong. * Separate core gradle plugins from third-party. This change also alphabetizes the plugins within each block to provide some semblance of structure. * Revert "Downgrade gradle wrapper to 7.0." (kolmafia#55) This reverts commit c77effb. Now that we're recommending people to use Java 17, they're having issues building. We need at least Gradle 7.1.1 to build with Java 17. * Updated to add JaCoCo test reports to Jenkins CI (kolmafia#54) * Update jenkinsfile Updated to add JaCoCo test reports to Jenkins CI * Update jenkinsfile * Correct merge mistakes brought by the rebase * Language server rebased spotless apply (#13) * Format the entirety of src/, test/. (kolmafia#50) This was done with `./gradlew spotlessApply`, followed by iterated application of sed to deal with my precommit hook complaining (tabs following space indentation as a result of commented-out blocks of code...) * Use Java 17 in actions (kolmafia#51) * Updating all dependencies except SwingX (kolmafia#58) ANTLR 3 Runtime 3.4 -> 3.52 FlatLaf 1.5 -> 1.6 FlatlLaf IntelliJ Themes Pack 1.5 -> 1.7 Add Flatlaf SwingX 1.6 JUnit Jupiter API 5.7.1 -> 5.8.0 JUnit Platform Launcher 1.7.2 -> 1.8.0 JUnit Vintage Engine 5.7.1 -> 5.8.0 Java Native Access 5.6.0 -> 5.9.0 Java Native Access Platform 5.6.0 -> 5.9.0 jansi 2.3.2 ->2.3.4 skipped SwingX, because it is causing conflicts with LockableListModel. working locally, passing tests. * Use browser launching code available since Java 1.6 (kolmafia#60) * Create CODEOWNERS file (kolmafia#61) This will just get github to automatically assign one of the team for reviewing PRs. I don't really recommend we use it in any more fine-grained way than this. * Run gradle checks for pull requests. (kolmafia#57) * Run gradle checks for pull requests. Among many other things, thiss will run spotlessCheck to make sure that code has been properly formatted. * Rename Run Checks -> Run Tests. Apparently our repository still requires _something_ to be called Run Tests. I guess I'd rather have a slightly misnamed task, than an empty or redundant task. * Require all files in test/root use newlines. (kolmafia#63) Our test setup specifies that the newline separator be \n so we can actually use golden files for testing output of scripts. This doesn't work so well when git mangles the golden files by adding newlines to them. Co-authored-by: heeheehee <heeheehee@users.sourceforge.net> * Deprecating ANT (kolmafia#41) * Deprecating ANT * Deprecating ANT * Enable gradle build cache (kolmafia#65) * Remove OSXAdapter and replace with Java built-ins (kolmafia#64) * Fix Taskbar API detection on unsupported platforms (kolmafia#67) * Only increment skill usage for successes. (kolmafia#56) * Only increment skill usage for successes. Thanks to phreddrickk @ kolmafia.us for reporting this issue. This adds an example of how one might use Mockito to inject a custom response. It's not super-well-documented, but I promise all the bits are necessary. In particular: we need to set session ID or else GenericRequest.run() will short-circuit, and we need to set max / current MP and "learn" the skill so UseSkillRequest.run() won't reject it before even passing it on to GenericRequest. These test cases follow the standard arrange / act / assert model. * Add a RequestTestBase for common setup / helpers. Right now, this provides some testing the happy path: you specify some response text for a spy, and we inject that data when you execute spy.run(). Important: in order for everything to work end-to-end, you need to provide a spy, not a regular mock. This allows us to exercise as much real behavior as possible. * Create README.md (kolmafia#62) * Create README.md Just our description and some useful status badges * Point release badge to the latest release page * Better revision capture for StaticEntity (kolmafia#68) * Better revision capture for StaticEntity * Check that attributes isn't null * Restore kolmafia-license.txt Somewhere we lost the license file and it's needed for About frame. This change returned the about box from the mists of time. If the correct answer is to remove the link for it, we coud do that, too. * Run github PR checks against latest mac/windows. (kolmafia#70) * Run checks against latest mac/windows. This also opts to use ubuntu-latest so we don't have to manually configure it in the future. Note that there are technically newer OSes than windows-latest and macos-latest at this time, but I anticipate that these labels will be automatically migrated over time (unlike a fixed label that will require periodic maintenance). * Add a placeholder dependent action. This is effectively a no-op, but we need to perform _some_ action in each step, and we need at least one step in each job. This allows us to get around the naming issues where the main branch expects a check with the name "Run Tests" to complete, while the matrix strategy modifies the name of each of the subjobs. * Comment on why we have this placeholder action. This also improves the stdout message from "1" to "All checks succeeded." because it doesn't cost anything to change a print statement... * Implicit Narrow (kolmafia#53) * Implicit Narrow Eliminate the last of the implicit narrowing by cast warnings. * Steps towards goal Added test, reformatted a source file. * Update SkillDatabaseTest.java * Handle implicit narrowing Create test for before/after comparison. Change code for easier access from test and then replace eventually incorrect calculation using Pow with something that is correct and won't overflow. * Fix Formatting Apparently telling IntelliJ to apply the Google format doesn't do what spotlessapply does. * Update SVNManager.java (kolmafia#71) * Remove Ant build (kolmafia#66) * Use grgit for updating the repo from origin/main. While we're at it, also remove the need for the git CLI for building from source. We do need this to be run in a git repository with a .git directory, though... And, remove the dependency on build.xml and default.properties, instead setting java.version in gradle.properties. * Delete build.xml, default.properties, jars. We haven't yet ported the javadoc task, exe / dmg building, or whatever experimental coverage viewer there was. We might not need any of those. * Also delete build.properties. I think this is only used in the Ant build. * Delete the download, toCopy parts of build.gradle. We no longer want to bundle jars explicitly (as we can / should instead rely on Gradle to handle dependency management), so there's no point in downloading them. * Only count revisions for commits on origin/main. This should prevent revision drift for those with lots of local commits. * Clean up build.gradle further. - Fix formatting, which doesn't seem to be addressed by spotless for some reason. - Remove svnRevList. We're steering hard into using git. - Remove redundant dependencies on gitRevList. pruneDist already relies on it, so things that rely on pruneDist shouldn't need to take an additional dependency. * Improve gitUpdate functionality. - Detect when no update is required. - Pull from the current tracked branch, as opposed to always origin/main. - If an update is required, "stash" the existing changes by creating a temp commit, rebase onto that temp commit, and then reset that commit. * Fix gradle formatting to use Eclipse style. I don't really care about the style as long as it actually works for nested blocks, which the previous one did not. * Format gradle files exclusively with greclipse(). I think this is causing check failures. * Add env variable for overriding the head commit. This should allow us to pass $GITHUB_SHA when we detect the number of revisions, rather than always using origin/main (which results in building the same revision multiple times if PRs are merged in quick succession). * Typo (kolmafia#73) Fixed a typo * Fix java source & target compatibility settings (kolmafia#75) * Link to the game in the README (kolmafia#76) * Test additional java releases (kolmafia#74) * Fix release build not using correct commit (kolmafia#72) * Option to pass commit to gradle * Fix release always using origin/main * Include commit in getRevision inputs * Remove Run Tests (I updated the branch protection) * switch to project property * Add info logging * Numeric cast (kolmafia#81) * Update BasementRequest.java Eliminate narrowing cast flagged by security * Update BasementRequest.java Clean ups suggested by IntelliJ's inspection. * Update BasementRequest.java spotlessApply * Update BasementRequest.java * Migrate JUnit4 tests to JUnit5 (kolmafia#78) * Migrate tests to JUnit 5. The most annoying ones were parameterized tests, since the structure changed appreciably. The others were mostly handled automatically by IntelliJ's migration actions, coupled with some search-and-replace to use assertions statically. Note that this is based on the `antless` branch, since I didn't want to deal with build.gradle merge conflicts shortly thereafter. * Remove leftover JUnit 4 dependencies. One test (SkillDatabaseTest) was added after I did the initial migration. This outright removes ErrorCollector from DataFileConsistencyTest, since it no longer exists in JUnit 5. We could continue using it, but I'd prefer not to if it means we can remove the dependency entirely. * Remove JUnit 4, vintage engine from build.gradle. To prevent backsliding in the future. * Publish test reports. While there's a comment insisting that it relies on pull_request_target (not pull_request), the example code for the action suggests using it with pull_request. Worth a shot, since I have no idea why a test is failing for Mac. * Convert CustomScriptTest to a parameterized test. I still have no idea why this is hanging in the MacOS build. ... Note after the fact: this is no longer hanging in the MacOS build, as of the migration to parameterized tests. I don't understand, but I'm not complaining. Could have been a transient issue with the github test runners. * Update BasementRequest.java (kolmafia#83) Somehow I failed to commit the most important line in a previous commit. * Add -M modified flag to version number inside KoLmafia code (for title bar, logs, about, CLI version, etc) (kolmafia#77) * handle dirty repo builds with explicit flag that matches build. * apply Spotless locally Co-authored-by: heeheehee-kolmafia <83351434+heeheehee-kolmafia@users.noreply.github.com> Co-authored-by: Joe Kirchoff <Kirchoff.Joseph.P@gmail.com> Co-authored-by: BadHorseMonkey <58645293+BadHorseMonkey@users.noreply.github.com> Co-authored-by: Samuel Gaus <sam@gaus.co.uk> Co-authored-by: Jamie Adams <82782908+jaadams5@users.noreply.github.com> Co-authored-by: heeheehee <heeheehee@users.sourceforge.net> Co-authored-by: Samuel Gaus <sam@gaus.co.uk> Co-authored-by: Rinn <Kirchoff.Joseph.P@gmail.com> Co-authored-by: BadHorseMonkey <58645293+BadHorseMonkey@users.noreply.github.com> Co-authored-by: Daniel Clements <fewyn@fewyn.net> Co-authored-by: Chris Midgley <midgleyc@gmail.com> Co-authored-by: Chris Midgley <chris.midgley@dunecomputers.co.uk> Co-authored-by: Jamie Adams <82782908+jaadams5@users.noreply.github.com> Co-authored-by: heeheehee-kolmafia <83351434+heeheehee-kolmafia@users.noreply.github.com> Co-authored-by: heeheehee <heeheehee@users.sourceforge.net>
* remove license from Command.java * Introduce the Range as a class extended by Token Also unveils the location-related methods we'll be working with * Add Location to Symbols * makeZeroWidthLocation(Position) * Introduce TypeReference Variable has VariableReference, and Function has FunctionCall, but Type had no class representing a reference to it (storing its own Location) * Merge the AggregateType constructors * change how type references are handled * Add Locations to Command classes other than Value * streamline parseNumber a bit more * Rename Run Checks -> Run Tests. Apparently our repository still requires _something_ to be called Run Tests. I guess I'd rather have a slightly misnamed task, than an empty or redundant task. * forgot the "@ Override" on those two * forgot the "@ Override" on those two * shift indentation to spaces * apply spotless * Merge with upstream/main (#15) * Use Java 9, add spotless w/ google java style. (#47) This is a proof-of-concept, and does not actually apply any formatting changes yet. I'd prefer to make that formatting change in a standalone commit. This also removes some comments in the middle of imports in KoLConstants.java because `./gradlew spotlessCheck` was yelling at me, because Google Java Format (gojf) doesn't like that. To reformat the code, you'll run `./gradlew spotlessApply`. This adds some flags so gojf continues to work for newer Java versions (16+). But for that, requires that the Java compiler be new enough to support exports, aka at least Java 9, which coincidentally is the highest version supported by OpenClover. We really should switch away from OpenClover to something like JaCoCo. (Coming soon!) * Implicit Narrowing part 2 (#46) Address additional implicit narrowing warnings * Migrate from OpenClover to JaCoCo for coverage (#48) * Use JaCoCo for coverage instead of Clover. They have support for Java > 9... * Remove Clover coverage comment. This branch, if merged, will mean that we're no longer using Clover, so it'll be moot. * Update jenkinsfile, too. I think this should work, but I could be wrong. * Separate core gradle plugins from third-party. This change also alphabetizes the plugins within each block to provide some semblance of structure. * Revert "Downgrade gradle wrapper to 7.0." (#55) This reverts commit c77effb. Now that we're recommending people to use Java 17, they're having issues building. We need at least Gradle 7.1.1 to build with Java 17. * Updated to add JaCoCo test reports to Jenkins CI (#54) * Update jenkinsfile Updated to add JaCoCo test reports to Jenkins CI * Update jenkinsfile * Format the entirety of src/, test/. (#50) This was done with `./gradlew spotlessApply`, followed by iterated application of sed to deal with my precommit hook complaining (tabs following space indentation as a result of commented-out blocks of code...) * Use Java 17 in actions (#51) * Updating all dependencies except SwingX (#58) ANTLR 3 Runtime 3.4 -> 3.52 FlatLaf 1.5 -> 1.6 FlatlLaf IntelliJ Themes Pack 1.5 -> 1.7 Add Flatlaf SwingX 1.6 JUnit Jupiter API 5.7.1 -> 5.8.0 JUnit Platform Launcher 1.7.2 -> 1.8.0 JUnit Vintage Engine 5.7.1 -> 5.8.0 Java Native Access 5.6.0 -> 5.9.0 Java Native Access Platform 5.6.0 -> 5.9.0 jansi 2.3.2 ->2.3.4 skipped SwingX, because it is causing conflicts with LockableListModel. working locally, passing tests. * Use browser launching code available since Java 1.6 (#60) * Create CODEOWNERS file (#61) This will just get github to automatically assign one of the team for reviewing PRs. I don't really recommend we use it in any more fine-grained way than this. * Run gradle checks for pull requests. (#57) * Run gradle checks for pull requests. Among many other things, thiss will run spotlessCheck to make sure that code has been properly formatted. * Rename Run Checks -> Run Tests. Apparently our repository still requires _something_ to be called Run Tests. I guess I'd rather have a slightly misnamed task, than an empty or redundant task. * Require all files in test/root use newlines. (#63) Our test setup specifies that the newline separator be \n so we can actually use golden files for testing output of scripts. This doesn't work so well when git mangles the golden files by adding newlines to them. Co-authored-by: heeheehee <heeheehee@users.sourceforge.net> * Deprecating ANT (#41) * Deprecating ANT * Deprecating ANT * Enable gradle build cache (#65) * Remove OSXAdapter and replace with Java built-ins (#64) * Fix Taskbar API detection on unsupported platforms (#67) * Only increment skill usage for successes. (#56) * Only increment skill usage for successes. Thanks to phreddrickk @ kolmafia.us for reporting this issue. This adds an example of how one might use Mockito to inject a custom response. It's not super-well-documented, but I promise all the bits are necessary. In particular: we need to set session ID or else GenericRequest.run() will short-circuit, and we need to set max / current MP and "learn" the skill so UseSkillRequest.run() won't reject it before even passing it on to GenericRequest. These test cases follow the standard arrange / act / assert model. * Add a RequestTestBase for common setup / helpers. Right now, this provides some testing the happy path: you specify some response text for a spy, and we inject that data when you execute spy.run(). Important: in order for everything to work end-to-end, you need to provide a spy, not a regular mock. This allows us to exercise as much real behavior as possible. * Create README.md (#62) * Create README.md Just our description and some useful status badges * Point release badge to the latest release page * Better revision capture for StaticEntity (#68) * Better revision capture for StaticEntity * Check that attributes isn't null * Restore kolmafia-license.txt Somewhere we lost the license file and it's needed for About frame. This change returned the about box from the mists of time. If the correct answer is to remove the link for it, we coud do that, too. * Run github PR checks against latest mac/windows. (#70) * Run checks against latest mac/windows. This also opts to use ubuntu-latest so we don't have to manually configure it in the future. Note that there are technically newer OSes than windows-latest and macos-latest at this time, but I anticipate that these labels will be automatically migrated over time (unlike a fixed label that will require periodic maintenance). * Add a placeholder dependent action. This is effectively a no-op, but we need to perform _some_ action in each step, and we need at least one step in each job. This allows us to get around the naming issues where the main branch expects a check with the name "Run Tests" to complete, while the matrix strategy modifies the name of each of the subjobs. * Comment on why we have this placeholder action. This also improves the stdout message from "1" to "All checks succeeded." because it doesn't cost anything to change a print statement... * Implicit Narrow (#53) * Implicit Narrow Eliminate the last of the implicit narrowing by cast warnings. * Steps towards goal Added test, reformatted a source file. * Update SkillDatabaseTest.java * Handle implicit narrowing Create test for before/after comparison. Change code for easier access from test and then replace eventually incorrect calculation using Pow with something that is correct and won't overflow. * Fix Formatting Apparently telling IntelliJ to apply the Google format doesn't do what spotlessapply does. * Update SVNManager.java (#71) * Remove Ant build (#66) * Use grgit for updating the repo from origin/main. While we're at it, also remove the need for the git CLI for building from source. We do need this to be run in a git repository with a .git directory, though... And, remove the dependency on build.xml and default.properties, instead setting java.version in gradle.properties. * Delete build.xml, default.properties, jars. We haven't yet ported the javadoc task, exe / dmg building, or whatever experimental coverage viewer there was. We might not need any of those. * Also delete build.properties. I think this is only used in the Ant build. * Delete the download, toCopy parts of build.gradle. We no longer want to bundle jars explicitly (as we can / should instead rely on Gradle to handle dependency management), so there's no point in downloading them. * Only count revisions for commits on origin/main. This should prevent revision drift for those with lots of local commits. * Clean up build.gradle further. - Fix formatting, which doesn't seem to be addressed by spotless for some reason. - Remove svnRevList. We're steering hard into using git. - Remove redundant dependencies on gitRevList. pruneDist already relies on it, so things that rely on pruneDist shouldn't need to take an additional dependency. * Improve gitUpdate functionality. - Detect when no update is required. - Pull from the current tracked branch, as opposed to always origin/main. - If an update is required, "stash" the existing changes by creating a temp commit, rebase onto that temp commit, and then reset that commit. * Fix gradle formatting to use Eclipse style. I don't really care about the style as long as it actually works for nested blocks, which the previous one did not. * Format gradle files exclusively with greclipse(). I think this is causing check failures. * Add env variable for overriding the head commit. This should allow us to pass $GITHUB_SHA when we detect the number of revisions, rather than always using origin/main (which results in building the same revision multiple times if PRs are merged in quick succession). * Typo (#73) Fixed a typo * Fix java source & target compatibility settings (#75) * Link to the game in the README (#76) * Test additional java releases (#74) * forgot the "@ Override" on those two * Fix release build not using correct commit (#72) * Option to pass commit to gradle * Fix release always using origin/main * Include commit in getRevision inputs * Remove Run Tests (I updated the branch protection) * switch to project property * Add info logging * Numeric cast (#81) * Update BasementRequest.java Eliminate narrowing cast flagged by security * Update BasementRequest.java Clean ups suggested by IntelliJ's inspection. * Update BasementRequest.java spotlessApply * Update BasementRequest.java * Migrate JUnit4 tests to JUnit5 (#78) * Migrate tests to JUnit 5. The most annoying ones were parameterized tests, since the structure changed appreciably. The others were mostly handled automatically by IntelliJ's migration actions, coupled with some search-and-replace to use assertions statically. Note that this is based on the `antless` branch, since I didn't want to deal with build.gradle merge conflicts shortly thereafter. * Remove leftover JUnit 4 dependencies. One test (SkillDatabaseTest) was added after I did the initial migration. This outright removes ErrorCollector from DataFileConsistencyTest, since it no longer exists in JUnit 5. We could continue using it, but I'd prefer not to if it means we can remove the dependency entirely. * Remove JUnit 4, vintage engine from build.gradle. To prevent backsliding in the future. * Publish test reports. While there's a comment insisting that it relies on pull_request_target (not pull_request), the example code for the action suggests using it with pull_request. Worth a shot, since I have no idea why a test is failing for Mac. * Convert CustomScriptTest to a parameterized test. I still have no idea why this is hanging in the MacOS build. ... Note after the fact: this is no longer hanging in the MacOS build, as of the migration to parameterized tests. I don't understand, but I'm not complaining. Could have been a transient issue with the github test runners. * Update BasementRequest.java (#83) Somehow I failed to commit the most important line in a previous commit. * Add -M modified flag to version number inside KoLmafia code (for title bar, logs, about, CLI version, etc) (#77) * handle dirty repo builds with explicit flag that matches build. * Delete svnrepo.json if it's bad. (#85) If the file throws a JSONException when we try to parse it, it's most likely not valid JSON. So, let's delete it. * Warn desktops who cannot launch a browser. This is really just very old linux installations... but mafia tend to be like that (#82) * Upgrade swingx to 1.6.1. (#79) * Upgrade swingx to 1.6.1. * Force update for each change to LockableListModel. AutoFilterTextField had some cases where it did not update this properly. * Remove param from LockableListModel.updateFilter. This parameter is error-prone and can readily cause the ListModel to get out of sync with anything that's watching it. We may see a slight performance hit from propagating updates more frequently. * Remove remaining traces of toCopy source set. We used this briefly for downloading jars for the Ant build. Since the Ant build is no more, this is no longer necessary. * Update AutoFilterTextField.java Co-authored-by: jaadams5 <82782908+jaadams5@users.noreply.github.com> * Add initial pass of CONTRIBUTING.md. (#87) * Add initial pass of CONTRIBUTING.md. There's a lot more room for expansion, but I think this is a good start. * Add section on testing, more links. * Fix spelling error. archetecture -> architecture. * Add some links to forum discussions. We've discussed development practices in a few public threads. The two I found covered CustomScriptTest and Gradle. * Wordsmith CONTRIBUTING.md some more. This rewrites the initial section to make it explicit that this describes the workflow / process, and not any specific tooling. This also reworks a couple of paragraphs under Guidelines. * Fix gradle jar. (#88) Veracity reported this breakage. Clearly I need to expand my pre-commit hook. * Replace line endings in test/ with LF. (#90) I guess this file snuck in before we finally added coverage for the directory in .gitattributes. * Create OS-specific builds via jpackage (#86) * Add bin task to Gradle. This creates a binary in build/KoLmafia/bin/KoLmafia. This change also adds a pruneBin task which is always run. I tried to figure out how to set up inputs / outputs, but jpackage insists on creating the output directory itself and will generate an error if said directory already exists. Follow-up changes will involve tinkering with a build matrix and looking at the resulting artifacts. * Copy-paste build to run ./gradlew bin. I'm not actually sure what the resulting files will look like on Windows / Mac, so here's to hoping I can actually see the artifacts before trying to publish them. * Upload all artifacts, then download them. This allows us to get all of the artifacts in one single container so we can theoretically publish them all at once later. * Add name, runs-on to release job. * Bundle artifacts as zips. We actually need everything in the generated directory, not just the runner binary. * Move code into build/releases, stop using wildcard. This attempted to save maybe one level of directory nesting, which I don't think actually matters in practice. * Stitch everything together. This splits the existing daily job into two targets: jar and release. `release` in turn depends on both `jar` and `bin`. 'bin' generates files Windows.zip, Linux.zip, and macOS.zip. * Remove extra newline. * Use jpackage-gradle-plugin instead of exec task. It doesn't feel appreciably different, but maybe it's more readable. * Improve portability of jpackage task. Mac creates the directory `build/releases/KoLmafia.app`, whereas Linux / Windows create `build/releases/KoLmafia`. jpackage will create all prerequisite directories, so it's fine with either deleting the inner directory, or its parent. * added appVersion, mac and windows package types, and mac icon * Add Windows icon for binary release. This uses the existing martini glass that was the KoLmelion icon once upon a time. Co-authored-by: BadHorseMonkey <58645293+BadHorseMonkey@users.noreply.github.com> * Reject risky character names. (#91) * Reject risky character names. We don't expect to ever receive character names containing '.', '/', or '\', but if we do, we shouldn't accept them, as this introduces a risk of directory traversal which can allow attackers to break out of $KOLMAFIA_ROOT. * Improve username validation. Name must be 3-30 characters long. Name must start with a letter. Name must be letters, numbers, spaces and underscores, and may not contain more than one space in a row. We're not handling requirements 2 or 4, but that's okay. It's better than what we were doing before. * Add explicit check for ".." to make CodeQL happier. The code seems like it checks for this specifically..... https://github.com/github/codeql/blob/main/java/ql/src/Security/CWE/CWE-022/TaintedPath.ql * Rip out jquery dependency. (#89) * Rip out jquery dependency. The features we rely on are native JS replacements as of IE9 or so. https://www.w3counter.com/trends says that maybe 0.1-0.2% of users are on IE8 and below, if even that. But also, this is only relevant if you aren't using KoL's action bar. * Fix translation error. If we're using document.getElementById and not document.querySelector, we don't need the # prefix, nor do we need to access into an array, since it only returns a single element (or null). * Basic support for vintner (and chateau de vinegar change) (#92) * Use consequences.txt for more item description parsing than we once were (#94) * Add tests for BasementRequest. (#84) * Add tests for BasementRequest. IMPORTANT: this change upgrades our required Java version to 11. This is the LTS release immediately following 8, so I don't anticipate as many breakages as the previous bump from 8 to 9. This isn't complete by any means (we're still missing elemental tests and HP/MP drain, as well as rewards), but it's a start. * Bump minimum Java required in README. * Set username instead of mocking Preferences. It turns out that mocking static methods of Preferences is slow for various reasons. That said, we can set preferences and read per-user defaults if we just set a username. It creates a default CCS file in test/root, but that's a small price to pay. This change also sets the min Java version in the readme to 11, since I'd missed that the first time around. Again, we don't have to bump the min version now if we don't want to; I'll just need to type a little bit more. * Revert Mockito dependency change. We're no longer trying to mock static methods, so we should be able to use mockito-core again. * Check for null in UseItemEqnueuePanel. (#95) We apparently need to do this every time we want to access a method of item. Worth refactoring on a later date so this is less dangerous. This change also resolves a mismatch between the item name (1950 Vampire Vintner wine) and its presence in inebriety.txt, modiiers.txt, and statuseffects.txt. For reasons, we only expanded DataFileConsistencyTest to cover inebriety.txt (since it best fit the model used by the existing check of equipment.txt). * Fix out of bounds error in Robocore (#98) * Finally got the guzzlr gold algorithm (#99) * Little mess up in ParserTest.java (#101) * Little mess up in ParserTest.java When making the transition to Junit5, one of the parenthesis was added *in a comment*! * need to remove that comma * correct the indentation * Turns out spotless was whining about more than just indentation * Revert "Upgrade swingx to 1.6.1. (#79)" (#100) * Revert "Upgrade swingx to 1.6.1. (#79)" This reverts commit 28affc9. * Remove toCopy again. We still want to get rid of this target, since building via Ant is no more. * Remove toCopy, part 2. When we re-added configurations, we put it in a different place. This stanza that was re-added is now entirely redundant. * shift indentation to spaces * apply spotless Co-authored-by: heeheehee-kolmafia <83351434+heeheehee-kolmafia@users.noreply.github.com> Co-authored-by: Jamie Adams <82782908+jaadams5@users.noreply.github.com> Co-authored-by: Daniel Clements <fewyn@fewyn.net> Co-authored-by: Joe Kirchoff <Kirchoff.Joseph.P@gmail.com> Co-authored-by: BadHorseMonkey <58645293+BadHorseMonkey@users.noreply.github.com> Co-authored-by: Samuel Gaus <sam@gaus.co.uk> Co-authored-by: heeheehee <heeheehee@users.sourceforge.net> * glitch that happened when resolving merge conflicts? * Turns out squash-and-merge was a bad idea so I had to do it again anyway... * Introduce LocatedValue<V>. This uses the static method *<V extends Value> LocatedValue<V> wrap(V value, Location location)* to allow Parser to attach a Location to Values. Values are too used outside of Parser for us to attach Locations to the natively. I would have much prefered to usea dynamic method for the wrapping i.e. *<value>.wrap(<location>)*, as opposed to the current *Value.wrap(<value>, <location>)*, but this, sadly, prevents us from adding a generic type to the resulting LocatedValue... (because of how hard it is to typecast *Value.this*) * rename fileURI * makeZeroWidthLocation(Position) was meant for the warning about missing comma between record initializers. Since we changed it to an error, it's no longer needed. * Must have been an issue when merging... * Evaluable!! \o/ * rename parseValue * Evaluable directly applied to upstream/main * rename Value.LocateValue * TypedNode interface * note about LocatedValue.evaluatesTo expecting an exact match * Rename LocatedValue, and add a small comment about it * spotless >:c * revert now-unnecessary change * spotlessApply * commit to reset tests, 1/2 * commit to reset tests, 2/2 * #removeAscensionHistoryRequestTest * #removeAscensionHistoryRequestTest * seems like these escaped the TypeNode fix * make range manipulation methods static * re-order some location manipulation methods * add mergeLocations(Location,Location) also make makeLocation(Location,Range) static * use Parser.mergeLocations outside of Parser * update Command.setLocation's comment * *rolls eyes* * *rolls eyes* * Move Line / Token / Comment to a separate file. * test positions (#17) * move getTokensContent to ParserTest * proof of concept * split testScriptValidity into 2 possible tests * reduce visibility of getTokensXXX they _maybe_ will end up getting called from outside, but it's so far away that it's better to just make them private for now anyway * make ValidScriptData.positions final not that it wasn't planned, but sure, let's take care of it now. * All were done by manually to ensure integrity this took so long :,( * rename getTokensX Sure, what's the harm * apply spotless * update tests to conform to the fact that Token is now a Range * fix endOfFile length * spotless * test Line's two new methods * wasn't seen as a conflict... * ForLoop implicit increment location changed at the last minute; this should have been in #239 ... * IncDec locations * FunctionCall locations * CompositeReference locations * Coercion function call locations * Assignment locations * Concatenate locations * Expression locations * spotless * Parser.mergeLocations(Command, Command) * spotless (°///°) Co-authored-by: heeheehee-kolmafia <83351434+heeheehee-kolmafia@users.noreply.github.com> Co-authored-by: Jamie Adams <82782908+jaadams5@users.noreply.github.com> Co-authored-by: Daniel Clements <fewyn@fewyn.net> Co-authored-by: Joe Kirchoff <Kirchoff.Joseph.P@gmail.com> Co-authored-by: BadHorseMonkey <58645293+BadHorseMonkey@users.noreply.github.com> Co-authored-by: Samuel Gaus <sam@gaus.co.uk> Co-authored-by: heeheehee <heeheehee@users.sourceforge.net>
Replaces #6