Skip to content

Commit

Permalink
Attempts to fix the leash unit test CI failures (tgstation#78157)
Browse files Browse the repository at this point in the history
## About The Pull Request

Fixes tgstation#77704

Fixes Skyrat-SS13/Skyrat-tg#23467
Fixes Skyrat-SS13/Skyrat-tg#23511

What it says on the tin. This has started causing CI failures with the
Tramstation test in almost every single PR downstream and it's gotten to
the point where it's become a major headache.

I don't know why it's happening only only on Tramstation, or why it's
happening at all-- but I will say it instantly began ramping up in
frequency immediately after merging
tgstation#75924 , which was the same
PR that was also causing the progressbar harddels previously to crop up
every time @LemonInTheDark .

This attempts to fix it by doing 2 things:

~~1) Make sure the `COMSIG_LEASH_PATH_COMPLETE` always gets sent, which
will prevent needless false 'timeouts' whenever every iteration of the
check_distance() loop returns early. Now it will only timeout when it
actually times out.~~

1) Increases the timeout timer to 80 seconds. Why so long? Well, I've
looked at a number of individual CI failures and it looks like it tries
to complete the path somewhere between 40-75 seconds after the time out
failure. Most of the time it should not take this long though and will
just finish right away as normal.

<details><summary>Timeouts with timestamps shown here so you can see
what I am talking about</summary>


![firefox_mjwj5LCEje](https://github.com/tgstation/tgstation/assets/13398309/376be09f-ab85-4362-a75f-156c76a5c901)


![firefox_St29Dis9Jh](https://github.com/tgstation/tgstation/assets/13398309/f5bfbb5f-fc24-404d-ba41-e84130885500)

</details>

<details><summary>And here</summary>


![firefox_joAs0qA0f6](https://github.com/tgstation/tgstation/assets/13398309/13955036-a120-4ea4-adcb-8c945d0695ab)


![firefox_WABTuH6Z1j](https://github.com/tgstation/tgstation/assets/13398309/222af6fb-8cb1-4ea1-97aa-c5785294da58)

</details>

<details><summary>And here</summary>


![firefox_BaZXVwFzPE](https://github.com/tgstation/tgstation/assets/13398309/c68f0c3a-553b-42ca-aef4-a04628c5abf0)


![firefox_qoMid37BUi](https://github.com/tgstation/tgstation/assets/13398309/8ce48a2d-3b21-4346-aa9f-654e66b2a6b7)

</details>

2) Moves the priority of the test down so it happens later, with the
other long tests. This might even fix the issue on its own--haven't seen
the test take more than 0.1s to complete so far in any of my runs. Maybe
things just needed some more time to load?

So far I've done about 10 test runs and haven't seen a single timeout
with these two changes. I'm fairly confident the issue is fixed but
we'll see.

Test functioning:


![image](https://github.com/tgstation/tgstation/assets/13398309/fd7e9cf4-3573-4971-bbee-2fe45f82dafb)

## Why It's Good For The Game

Stops a PR blocking spurious CI failure.

## Changelog

:cl:
fix: leash unit test will time out less often and increases the timer
until it is considered 'timed out', to reduce false CI failures
/:cl:
  • Loading branch information
vinylspiders authored Sep 6, 2023
1 parent b511213 commit 3e59765
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion code/modules/unit_tests/leash.dm
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/datum/unit_test/leash
abstract_type = /datum/unit_test/leash
priority = TEST_LONGER

var/atom/movable/owner
var/atom/movable/pet
Expand Down Expand Up @@ -55,7 +56,7 @@
var/timed_out = FALSE

/datum/leash_wait/New()
addtimer(VARSET_CALLBACK(src, timed_out, TRUE), 1 SECONDS)
addtimer(VARSET_CALLBACK(src, timed_out, TRUE), 80 SECONDS)

/datum/leash_wait/proc/completed()
completed = TRUE
Expand Down

0 comments on commit 3e59765

Please sign in to comment.