Skip to content

Move to GH actions #58

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

Merged
merged 10 commits into from
Jun 7, 2022
Merged

Move to GH actions #58

merged 10 commits into from
Jun 7, 2022

Conversation

MrLotU
Copy link
Collaborator

@MrLotU MrLotU commented Aug 4, 2021

Move to GH Actions instead of Circle CI. Also adds Swift 5.3 and 5.4 as test targets.

Checklist

  • The provided tests still run.
  • I've created new tests where needed.
  • I've updated the documentation if necessary.

@@ -53,6 +53,7 @@ final class GaugeTests: XCTestCase {
""")
}

#if os(Linux)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not super happy with this solution, but from some checking the MacOS runners make this sleep last somewhere between 0.05 and 0.1 seconds, which makes these tests incredibly flaky. Locally they run just fine.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Any input appreciated. CC @ktoso

Copy link
Collaborator

Choose a reason for hiding this comment

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

We can introduce the concept of time dilation. We used to do this all the time in Akka where we had very slow CI machines and had to adjust timeouts on them for all tests. It's documented a bit here: https://doc.akka.io/docs/akka/current/testing.html#accounting-for-slow-test-systems

This means that we'd do: delay.dilated where dilated would be

var dilated: Double { 
  if <DETECT IF ON SLOW RUNTIME> 
    return self * /*dilationFactor (e.g: */ 2 // easiest to have this be set in an env var
  else 
  return self
}

So in the CI, env we'd just set the TEST_TIME_DILATION_FACTOR=10 or something that makes it happy :-)

WDYT?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If I understand correctly this would turn this test from sleeping for 0.05 seconds, to sleeping for 50. Which would than require to also update the asserts based on the time dilation, correct? In that case, would that not only make the problem worse?

In this case it's not a timeout that's messing things up, it's measuring the time a Thread.sleep takes, and on slow CI machines it takes longer than it's supposed to, resulting in "wrong" metrics.

Copy link
Collaborator

@ktoso ktoso Aug 24, 2021

Choose a reason for hiding this comment

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

Yeah I wasn't all to clear I guess. In such tests we need to adjust the "expectation" rather than the sleep -- so we'd apply the dilated to the expectation that the time was smaller than something.

All such sleepy tests are pretty meh so let's just give it a good best effort :-)

@MrLotU MrLotU requested a review from ktoso August 24, 2021 08:56
@MrLotU
Copy link
Collaborator Author

MrLotU commented Jun 3, 2022

@ktoso Decided on a new strategy to do these time based tests. Thoughts? 😄

@MrLotU MrLotU merged commit 36740d1 into master Jun 7, 2022
@MrLotU MrLotU deleted the gh_actions branch June 7, 2022 11:58
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.

2 participants