Standardize inputs and refactor workflows

[ChipWolf]

This PR refines several things:

• Various grammar/formatting nitpicks
• Removal of various redundant workflows, consolidated into fewer, easier to maintain workflows
• Replacing manual download commands with the robinraju/release-downloader action, so the action leverages the runner's GITHUB_TOKEN against the API as standard
• Grant execute permission to gradlew files and removed redundant workflow steps
• Added caching for CI builds and mc/modloader downloads

[3arthqu4ke]

Looks awesome, do you think there is a way to provide caching for the release downloads?

[ChipWolf]

Looks awesome, do you think there is a way to provide caching for the release downloads?

That's what I've done in our repo, I just need to get it into this branch

Only thing I need to consider is that many people (including ourselves) are using custom runners like Buildjet/Blacksmith/WarpBuild/Depot etc., and some of them have their own caching actions, so I should probably allow that to be provided as an input

Setup Java as you can see I moved out of the action because that seems to be the standard for most other workflows including some of GitHub's own, and it gives people some flexibility (especially if they're running this as part of a build & test job which would require Java already)

I was a little overwhlemed by the huge number of actions in this repo, many of which don't appear to be used, so I'd like to consolidate those in this PR also

[ChipWolf]

Question @3arthqu4ke, why does x11-xserver-utils need to be installed?
I note our workflows appear to run fine without it, is there some edge cases that need it, or is it perhaps already installed in the runner?

Also, I note there's effectively 2 different "products" in this repo, the mc-runtime-test mod, and the action by the same name (which primary function is to run headlessmc, but depends on mc-runtime-test), do you reckon there might be some value in the mod being in its own repo?

[ChipWolf]

Alright, that's the majority of what I wanted to achieve done

The scope has increased somewhat and I'll update my docs, but I've pushed regardless

The entire build + run matrix takes under 4 minutes now after the cache is seeded

[3arthqu4ke]

I was a little overwhlemed by the huge number of actions in this repo, many of which don't appear to be used, so I'd like to consolidate those in this PR also

Yes many can just be thrown out.

Question @3arthqu4ke, why does x11-xserver-utils need to be installed? I note our workflows appear to run fine without it, is there some edge cases that need it, or is it perhaps already installed in the runner?

This is an edge case for 1.12.2, with lwjgl-2.9.4 and xvfb, I am not sure if older lwjgl versions are affected.
In that version lwjgl crashes because it checks xrandr for displays, which is not installed on the runners, x11-xserver-utils provides it.
https://github.com/3arthqu4ke/mc-runtime-test/actions/runs/11713260682/job/32674624091#step:21:3209

Also, I note there's effectively 2 different "products" in this repo, the mc-runtime-test mod, and the action by the same name (which primary function is to headlessmc, but depends on mc-runtime-test), do you reckon there might be some value in the mod being in its own repo?

Hmmm, good question. Value in putting the mod in its own repository would probably be that it would make workflows in this repository much faster, because the run workflow does not need to build every single mod version every time.

On the other hand if I release a new version of the mod it means I also need to release a new version of the action, having it in one place makes that easier, but that is a nitpick.

[ChipWolf]

Yes many can just be thrown out.

✅ Already sorted!

---

This is an edge case for 1.12.2, with lwjgl-2.9.4 and xvfb, I am not sure if older lwjgl versions are affected.
In that version lwjgl crashes because it checks xrandr for displays, which is not installed on the runners, x11-xserver-utils provides it.

• Got it, makes sense. I’ll update the workflow to install x11-xserver-utils only when it’s needed, so it doesn’t add unnecessary overhead for other versions. We could also try this variable: https://github.com/LWJGL/lwjgl/blob/2df01dd762e20ca0871edb75daf670ccacc89b60/src/java/org/lwjgl/opengl/LinuxDisplay.java#L214

• I can't replicate this; see https://github.com/marcusk-studio/mc-runtime-test/actions/runs/12085945442/job/33704307459

---

On the other hand if I release a new version of the mod it means I also need to release a new version of the action, having it in one place makes that easier, but that is a nitpick.

Keeping them together:

• We could just use the latest released version for workflows and let RenovateBot handle dependency updates automatically when a new release is out

Splitting them up:

• RenovateBot + Release Please could automate updates and semantic versioning (you’d need to adopt Conventional Commits).

Honestly, I’m leaning toward splitting them. The mod feels pretty repetitive right now (lots of duplicated code between version folders), and separating it would make it easier to DRY up without affecting the action workflows. Plus, I’m not a Java dev, so having it in its own space would make the action easier to work on.

[3arthqu4ke]

This is an edge case for 1.12.2, with lwjgl-2.9.4 and xvfb, I am not sure if older lwjgl versions are affected.
In that version lwjgl crashes because it checks xrandr for displays, which is not installed on the runners, x11-xserver-utils provides it.

* ~Got it, makes sense. I’ll update the workflow to install `x11-xserver-utils` only when it’s needed, so it doesn’t add unnecessary overhead for other versions. We could also try this variable: https://github.com/LWJGL/lwjgl/blob/2df01dd762e20ca0871edb75daf670ccacc89b60/src/java/org/lwjgl/opengl/LinuxDisplay.java#L214~

* I can't replicate this; see https://github.com/marcusk-studio/mc-runtime-test/actions/runs/12085945442/job/33704307459

I have tried the LWJGL_DISABLE_XRANDR variable and it did not work, lwjgl tries to fallback to XF86VIDMODE which also fails and then crashes.

Odd, I will look into this. Your runner is newer than the one used for my workflow but besides that I have no clue.

Honestly, I’m leaning toward splitting them. The mod feels pretty repetitive right now (lots of duplicated code between version folders), and separating it would make it easier to DRY up without affecting the action workflows. Plus, I’m not a Java dev, so having it in its own space would make the action easier to work on.

Yeah, I also tend to splitting them, I have plans for the duplicate code. While supporting many different minecraft and modloader versions is difficult, it could be possible with a Java Pre-processor #8

[3arthqu4ke]

I can't replicate this; see https://github.com/marcusk-studio/mc-runtime-test/actions/runs/12085945442/job/33704307459

https://github.com/marcusk-studio/mc-runtime-test/actions/runs/12085945442/job/33704307750#step:6:583
Here you are not specifying -Dhmc.check.xvfb=true and you are in offline mode so headlessmc will automatically run with -lwjgl.

In the xvfb-run workflow the -lwjgl flag is used.
https://github.com/marcusk-studio/mc-runtime-test/actions/runs/12085945442/job/33704307459#step:6:569

[3arthqu4ke]

https://github.com/marcusk-studio/mc-runtime-test/blob/e42dbcc8274572ba4ec533a6068cc7b6eb5d3732/.github/workflows/run-matrix.yml#L178

I think this is the wrong way around? If you use xvfb you do not want to add -lwjgl?

[ChipWolf]

https://github.com/marcusk-studio/mc-runtime-test/blob/e42dbcc8274572ba4ec533a6068cc7b6eb5d3732/.github/workflows/run-matrix.yml#L178

I think this is the wrong way around? If you use xvfb you do not want to add -lwjgl?

I inverted the logic on this test run, same thing https://github.com/marcusk-studio/mc-runtime-test/actions/runs/12104807693

I also tried both temurin and adopt to try and replicate the crash, no dice.

---

In the name of progress, I'm inclined to polish and get this PR out of WIP so we can get it merged and make more progress. I don't want any more scope creep on this PR.

I'll add Release-Please and Renovate in a separate PR

[3arthqu4ke]

I inverted the logic on this test run, same thing https://github.com/marcusk-studio/mc-runtime-test/actions/runs/12104807693

The lwjgl flag is still there in the xvfb-run.
https://github.com/marcusk-studio/mc-runtime-test/actions/runs/12104807693/job/33748650272#step:6:104

Its there in both runs
https://github.com/marcusk-studio/mc-runtime-test/actions/runs/12104807693/job/33748650347

Maybe the matrix input is interpreted as a string?
Could not find the exact documentation but maybe try this:
https://github.com/orgs/community/discussions/25555#discussioncomment-3248310

In the name of progress, I'm inclined to polish and get this PR out of WIP so we can get it merged and make more progress. I don't want any more scope creep on this PR.

I'll add Release-Please and Renovate in a separate PR

👍

[ChipWolf]

The lwjgl flag is still there in the xvfb-run.

The issue with the lwjgl flag in xvfb-run came down to how GitHub Actions handles its syntax. Originally, I tried this:

${{ matrix.xvfb && '' || '-lwjgl' }}

Normally, you'd expect this to work like a ternary operator:

${{ condition && 'if true' || 'if false' }}

But in this case, even when matrix.xvfb was true, the empty string ('') was treated as false. So the || '-lwjgl' part kicked in. Turns out, GitHub Actions doesn’t support a proper ternary operator, and that empty string messed things up.

To patch the behavior, I flipped the logic:

${{ !matrix.xvfb && '-lwjgl' || '' }}

This works because:

1. !matrix.xvfb checks if matrix.xvfb is false.
2. If it is, you get '-lwjgl'.
3. If not, it falls back to '' and as there's no operator following the string, it becomes the final output.

Now the empty string is in the right spot, and everything works as expected. It’s a simple tweak, but it gets around the weirdness with GitHub Actions’ syntax.

I'll bring that change over into this branch soon
https://github.com/marcusk-studio/mc-runtime-test/actions/runs/12105950159/job/33751158630

[3arthqu4ke]

The Build, Test, Release workflow just failed due to 1.7.10 timing out (1.7.10 is a bit flaky sometimes it waits for the player indefinitely, not sure why).

But when I tried to re-run the failed tests, they all failed because they were not able to find the artifacts uploaded in the Build job. Probably because of the Cleanup Step that always runs deleted them.

This is just an observation. I dont think this is necessarily bad because I wouldnt know how to do it differently while having a cleanup step. I like that the artifacts are deleted, but if a test fails we need to re-run all tests, instead of the cancelled/failed ones.

[3arthqu4ke]

On timeout: headlessmc handles retries and retries on timeout would also be great. Just opening the discussion to the question if handling the timeout in headlessmc would make sense, or handling retries in mc-runtime-test would make sense.

The reason I put retries on the headlessmc side for now was to distinguish between headlessmc configuration errors and minecraft errors but not sure.

Another question that is left to the user is, if they even want retries. It could hide flakyness issues in the users own application, but on the other hand there are flaky Minecraft versions (1.7.10, 1.19.1-1.19.3, 1.20.1-4 nightconfig crashes).

[3arthqu4ke]

Ah and this time the 1.19.3 test just timed out, just as I wrote about the 1.19.3 flakyness.
I think I will need to look into this on the mc-runtime-test mod side as well.

Edit: yeah I remember now why I commented out the 1.19-1.19.3 tests in headlessmc, these versions often get stuck on the CreateWorldScreen. https://github.com/3arthqu4ke/headlessmc/blob/9c3551e470862a0e25b988011d992ff0bbc87766/.github/workflows/run-matrix-xvfb.yml#L64

[3arthqu4ke]

This is just an observation. I dont think this is necessarily bad because I wouldnt know how to do it differently while having a cleanup step. I like that the artifacts are deleted, but if a test fails we need to re-run all tests, instead of the cancelled/failed ones.

Actually the only idea I just had would be to have a conditional confirmation step before the cleanup step if the run step failed.
But only if you want to, this is just a small optimization and not really in the scope.

[ChipWolf]

This is just an observation. I dont think this is necessarily bad because I wouldnt know how to do it differently while having a cleanup step. I like that the artifacts are deleted, but if a test fails we need to re-run all tests, instead of the cancelled/failed ones.

Perhaps we should only clean up artifacts from passed tests

I'm working on selective builds right now based on the content of a PR, so in those instances I believe we shouldn't remove the artifacts either. See here: https://github.com/marcusk-studio/mc-runtime-test/actions/runs/12111015708

[ChipWolf]

Alright this is about as polished as I can manage for now @3arthqu4ke, I'll raise another PR after this is in for the dependency updates and semantic versioning etc.

I'm sure there'll be some teething issues, but it'll be simple enough to squash them now

[ChipWolf]

DRY this, we should avoid referencing the filename twice.

[ChipWolf]

DRY this, we should avoid referencing the filename twice.

[3arthqu4ke]

Could you rebase on top of main real quick, please? I think I fixed the 1.19 issues, that way I can run the workflows for this PR without having to restart them if 1.19 fails.

[3arthqu4ke]

Thanks a lot, this is awesome!

[3arthqu4ke]

Workflows now no longer fail fast if something in the run step goes wrong, but I guess that was to be expected?

[3arthqu4ke]

Can be merged

[ChipWolf]

Workflows now no longer fail fast if something in the run step goes wrong, but I guess that was to be expected?

Yeah that was a last minute choice
My thinking is if you've changed various things you probably want to see all the problems in one go

Equally you probably want a complete picture of which configurations work

[ChipWolf]

Could you rebase on top of main real quick, please? I think I fixed the 1.19 issues, that way I can run the workflows for this PR without having to restart them if 1.19 fails.

Will do

[3arthqu4ke]

Could you rebase on top of main real quick, please? I think I fixed the 1.19 issues, that way I can run the workflows for this PR without having to restart them if 1.19 fails.

Will do

No need to, workflows ran successfully, will merge now, Just wanted clarification on my comment. Perfekt, great Work!

[3arthqu4ke]

Alright this is about as polished as I can manage for now @3arthqu4ke, I'll raise another PR after this is in for the dependency updates and semantic versioning etc.

I'm sure there'll be some teething issues, but it'll be simple enough to squash them now

Should I just bump everything to 3.0.0 and release, or what did you have in mind?

[ChipWolf]

Alright this is about as polished as I can manage for now @3arthqu4ke, I'll raise another PR after this is in for the dependency updates and semantic versioning etc.
I'm sure there'll be some teething issues, but it'll be simple enough to squash them now

Should I just bump everything to 3.0.0 and release, or what did you have in mind?

Let me get the renovate/release-please PR in then we can run the 3.0.0 release automatically