Skip to content

Enable PR runner for integration tests - #5874

Merged
WatchTheFort merged 20 commits into
beyond-all-reason:masterfrom
NortySpock:pr-test-runner
Oct 30, 2025
Merged

Enable PR runner for integration tests#5874
WatchTheFort merged 20 commits into
beyond-all-reason:masterfrom
NortySpock:pr-test-runner

Conversation

@NortySpock

@NortySpock NortySpock commented Oct 1, 2025

Copy link
Copy Markdown
Contributor

Work done

  • rebased PR PR test runner #4140 on master
  • removed a test for air repair pads
  • skipped two tests that were failing (hope that's ok so that progress can be made on getting CICD working)
  • In theory this should run tests in GitHub Actions (we'll see if it still works)
  • Sets tests to run on the "manual-linux-test-engine" engine from the release config.json
    • recoil_2025.06.06
    • ...which still seems old relative to master... are there newer releases we should be targeting? Or is "manual-linux-test-engine" always going to be the correct one to target?

Test steps

  • Write the steps needed to test out the changes. Include the expected result.

Test run: docker compose -f tools/headless_testing/docker-compose.yml up

BEFORE:

no tests are run

AFTER:

tests run via GitHub Actions, GitHub Action log output now includes:

[Test Runner] =====TEST RESULTS=====
[Test Runner] SKIP: cmd_blueprint/test_cmd_blueprint_filter.lua [0 frames] [0 ms]
[Test Runner] SKIP: cmd_blueprint/test_cmd_blueprint_line.lua [0 frames] [0 ms]
[Test Runner] PASS: cmd_blueprint/test_cmd_blueprint_single.lua [53 frames] [1764 ms]
[Test Runner] PASS: cmd_stop_selfd/test_cmd_stop_selfd.lua [7 frames] [234 ms]
[Test Runner] PASS: critters/test_critters.lua [315 frames] [4621 ms]
[Test Runner] PASS: gui_selfd_icons/test_gui_selfd_icons_armpw.lua [9 frames] [292 ms]
[Test Runner] PASS: gui_selfd_icons/test_gui_selfd_icons_armvp.lua [9 frames] [297 ms]
[Test Runner] SKIP: mex-building/pregame_mex_queue.lua [0 frames] [1 ms]
[Test Runner] SKIP: mex-building/pregame_mex_snap.lua [0 frames] [0 ms]
[Test Runner] SKIP: select_api/compare_to_spring.lua [0 frames] [0 ms]
[Test Runner] PASS: selftests/test_assertions.lua [3 frames] [101 ms]
[Test Runner] PASS: selftests/test_callins.lua [40 frames] [1328.99988 ms]
[Test Runner] SKIP: weapondefs/test_flighttime.lua [0 frames] [0 ms]
[Test Runner] SKIP: infolog [0 frames] [0 ms]

That last infolog one seems to be due to the lua code calling deprecated engine features, which is interesting and probably needs to be looked into...

I really want to properly address that infolog one, those look like some useful things to catch in the logs and fix, but I don't want "slightly better test coverage" to block "having a CI test runner".

LLM disclosure statement:

I did not use an LLM for any of my contribution to this branch. This branch was rebased from Saurtron's work, who did the heavy-lifting.

@NortySpock
NortySpock marked this pull request as ready for review October 1, 2025 12:22
Comment thread tools/headless_testing/springsettings.cfg Outdated
@NortySpock
NortySpock requested a review from sprunk October 3, 2025 00:54
@NortySpock NortySpock changed the title Enable PR test runner Enable PR runner for integration tests Oct 13, 2025
@NortySpock
NortySpock marked this pull request as draft October 15, 2025 17:01
@NortySpock
NortySpock marked this pull request as ready for review October 22, 2025 01:34
@NortySpock

NortySpock commented Oct 22, 2025

Copy link
Copy Markdown
Contributor Author

Rebased on top of master (commit 1911aed , PR was #5986 )

Integration tests (the ones that are not skipped) are passing, so this is ready for re-review.

Comment thread common/testing/infologtest.lua
Comment thread tools/headless_testing/startscript.txt Outdated
@NortySpock
NortySpock marked this pull request as draft October 22, 2025 13:25
@NortySpock
NortySpock requested a review from sprunk October 23, 2025 01:37
@NortySpock
NortySpock marked this pull request as ready for review October 23, 2025 01:37

@sprunk sprunk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Lua code parts look good, no idea about the github workflow or the policy on skipping tests so somebody else should also take a look at those.

@NortySpock

Copy link
Copy Markdown
Contributor Author

I did (just now) run the GitHub action on my own repository:

link to action run: https://github.com/NortySpock/Beyond-All-Reason/actions/runs/18767680295/job/53545744940

Image of the summary of the run itself:
image

Image of processing the output of the run into test results:

image

So the GitHub action side of it does work and produces a test output.

@NortySpock

NortySpock commented Oct 24, 2025

Copy link
Copy Markdown
Contributor Author

My recommendation on testing policy would be:

  1. Green: We want the green checkmark of a passing test run on the PR, as this shows things are in working order.
  2. Fix until green: Focus on fixing / hotfixing failing tests on the master branch and stable branch before merging in new features. you don't want people to get in the habit of ignoring failing tests
  3. 🗑 Remove tests if they are no longer relevant... they are there to guide and assist, not to be a straitjacket.
  4. Keep only a handful of these integration tests, but make them a good barometer of health. [1]

(this comment was hand-written by a human, I just wanted to use emoji to drive my point home.)

[1] the meat of the tests will likely be in the unit tests, not here in these integration tests.
[2] I get that this is a hobby project and a labor of love. I do not want to crush anyone's spirit with testing requirements. ....I just don't think it makes sense to manually test all changes on a project of this size.

@NortySpock

NortySpock commented Oct 24, 2025

Copy link
Copy Markdown
Contributor Author

For an example of how this fails, I re-enabled a failing test temporarily and allowed the test runner to kick again. The tests on the commit fail with this little red x next to the commit:

image

...And developers can click into the modal, click the failing test, and show the output to get the lines that failed in the infolog.

image

(Again, all props to Saurtron, they did the hard work of making this work and look nice; I'm just trying to get this merged in)

@NortySpock

Copy link
Copy Markdown
Contributor Author

@Damgam would you be willing to merge this? I've passed a review from a wizard and I've demonstrated the behavior.

@przystuj

Copy link
Copy Markdown
Contributor

I know nothing about GitHub actions etc, but having tests run on the PR level is amazing, please get it in :D

@WatchTheFort
WatchTheFort merged commit 0330e7e into beyond-all-reason:master Oct 30, 2025
1 check passed
@NortySpock

Copy link
Copy Markdown
Contributor Author

Just a followup here, I do see some of the tests now failing in the master branch. (Yay tests! Boo test failures!)

I intend to triage them by Monday at the latest and get a followup PR to try to get the tests to green.

Love that we finally have integration tests in CI so that developers can automate some of their testing!

@NortySpock NortySpock mentioned this pull request Oct 30, 2025
Xehrath pushed a commit to Xehrath/Beyond-All-Reason that referenced this pull request Jan 31, 2026
- rebased  PR beyond-all-reason#4140 on master
- removed a test for air repair pads
- skipped two tests that were failing (hope that's ok so that progress can be made on getting CICD working)
- In theory this should run tests in GitHub Actions (we'll see if it still works)
- Sets tests to run on the "manual-linux-test-engine" engine from the release config.json
  - recoil_2025.06.06
- ...which still seems old relative to master... are there newer releases we should be targeting? Or is "manual-linux-test-engine" always going to be the correct one to target?
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