Skip to content

Fix JobWeightTest causing failures - #5380

Merged
redmushie merged 5 commits into
ss14Starlight:starlight-devfrom
redmushie:bugfix/round-start-test
Jul 28, 2026
Merged

Fix JobWeightTest causing failures#5380
redmushie merged 5 commits into
ss14Starlight:starlight-devfrom
redmushie:bugfix/round-start-test

Conversation

@redmushie

Copy link
Copy Markdown
Contributor

Short description

JobWeightTest disconnects the pair, but does not dirty the pair. The rest of the tests rely on them being connected

Why we need to add this

Failing lots, e.g. on #5169

Media (Video/Screenshots)

Checks

  • I do not require assistance to complete the PR.
  • Before posting/requesting review of a PR, I have verified that the changes work.
  • I have added screenshots/videos of the changes, or this PR does not change in-game mechanics.
  • I affirm that my changes are licensed under the MIT License and grant permission for use in this repository under its conditions.

Not user facing, no CL

@redmushie
redmushie requested a review from a team July 28, 2026 10:59
@github-actions github-actions Bot added size/S S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. Changes: C# S: Needs Review Status: Requires additional reviews before being fully accepted. Not to be replaced by S: Approved. labels Jul 28, 2026
@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The integration test fixture adds a Disconnected pool configuration with Dirty = true and applies it to JobWeightTest using the PairConfig attribute.

Changes

Job weight test configuration

Layer / File(s) Summary
Apply dirty pool settings to JobWeightTest
Content.IntegrationTests/Tests/Round/JobTest.cs
Adds the PairConfig import, defines Disconnected pool settings with Dirty = true, and applies them to JobWeightTest.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Suggested reviewers: rinary1

🚥 Pre-merge checks | ✅ 8
✅ Passed checks (8 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
No Magic Numbers ✅ Passed Nice fix: the patch adds only PairConfig/Dirty wiring and no new numeric literals or thresholds.
No Hardcoded Ecs Parameters ✅ Passed PASS: The PR only adjusts an integration test fixture; no ECS systems or component defaults were introduced, so there’s no hardcoded ECS parameter logic to flag.
Avoid Service Locator ✅ Passed Nice, clean test-only change: no new service-locator usage was introduced; it only adds a PairConfig and dirty-pair settings.
Title check ✅ Passed The title clearly matches the main change: fixing JobWeightTest-related failures.
Description check ✅ Passed The description accurately describes the test fix and the connected-state issue it addresses.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Jul 28, 2026
Comment thread Content.IntegrationTests/Tests/Round/JobTest.cs Outdated
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jul 28, 2026
@Mackamuir

Copy link
Copy Markdown
Contributor

Also this test is evil and we have tried to fix it many times apparently
#4487

@redmushie redmushie changed the title Fix shard 6, make JobWeightTest dirty the pair Fix JobWeightTest causing failures Jul 28, 2026
@starlightgithub starlightgithub Bot added S: Approved Status: Reviewed and approved by at least one maintainer or dev; a PR may require another approval. and removed S: Needs Review Status: Requires additional reviews before being fully accepted. Not to be replaced by S: Approved. labels Jul 28, 2026
@redmushie
redmushie added this pull request to the merge queue Jul 28, 2026
Merged via the queue into ss14Starlight:starlight-dev with commit 24c86e1 Jul 28, 2026
26 checks passed
@redmushie
redmushie deleted the bugfix/round-start-test branch July 28, 2026 17:44
Limerent-Sun pushed a commit to Blimpuf-Station/BlimpufStation that referenced this pull request Aug 5, 2026
## Short description
<!-- What do you propose to change with your PR? -->

JobWeightTest disconnects the pair, but does not dirty the pair. The
rest of the tests rely on them being connected

## Why we need to add this
<!-- What is the reason for adding these changes? Please post links to
Discussions as well as Bug Reports here. Please describe how this will
change the game balance. -->

Failing lots, e.g. on
ss14Starlight#5169

## Media (Video/Screenshots)
<!--
If your PR contains in-game changes you must provide screenshots/videos
of the changes.
-->

## Checks
<!-- check boxes for faster reviewing of your PR -->

- [x] I do not require assistance to complete the PR.
- [ ] Before posting/requesting review of a PR, I have verified that the
changes work.
- [x] I have added screenshots/videos of the changes, or this PR does
not change in-game mechanics.
- [x] I affirm that my changes are licensed under the [MIT
License](https://github.com/ss14Starlight/space-station-14/blob/Starlight/LICENSE.TXT)
and grant permission for use in this repository under its conditions.

Not user facing, no CL
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Changes: C# S: Approved Status: Reviewed and approved by at least one maintainer or dev; a PR may require another approval. S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. size/XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants