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.

Sign up for GitHub

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

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
7c220ff
Move to GH actions
MrLotU Aug 4, 2021
db0dad8
Make time based tests less flakey
MrLotU Aug 24, 2021
731ba3e
Merge branch 'master' into gh_actions
MrLotU Aug 24, 2021
e139f9e
Add debug output to failing tests
MrLotU Aug 24, 2021
89d36a7
Don't run timing based tests on MacOS
MrLotU Aug 24, 2021
8f58425
Merge branch 'master' into gh_actions
MrLotU Jun 3, 2022
89f94d7
chore: Work around time-flakyness
MrLotU Jun 3, 2022
2652588
chore: Re add swift 5.2 and 5.3
MrLotU Jun 3, 2022
6e6a4ff
chore: Update the histogram test to allow for even more varying CI times
MrLotU Jun 3, 2022
d5ab6d2
Merge branch 'master' into gh_actions
MrLotU Jun 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
name: tests
on: { pull_request: {} }

jobs:
linux:
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
docker: ['swift:5.2-focal', 'swift:5.3-focal', 'swift:5.4-focal']
container: ${{ matrix.docker }}
steps:
- name: Check out SwiftPrometheus
uses: actions/checkout@v2
- name: Run tests with Thread Sanitizer
timeout-minutes: 30
run: swift test --enable-test-discovery --sanitize=thread
macos:
runs-on: macos-latest
strategy:
fail-fast: false
matrix:
toolchain: ['latest', 'latest-stable']
steps:
- name: Select toolchain
uses: maxim-lobanov/[email protected]
with:
xcode-version: ${{ matrix.toolchain }}
- name: Check out SwiftPrometheus
uses: actions/checkout@v2
- name: Run tests with Thread Sanitizer
timeout-minutes: 30
run: swift test --enable-test-discovery --sanitize=thread
2 changes: 2 additions & 0 deletions Tests/SwiftPrometheusTests/GaugeTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 •
edited
Loading

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 :-)

func testGaugeTime() {
let gauge = prom.createGauge(forType: Double.self, named: "my_gauge")
let delay = 0.05
Expand All @@ -65,6 +66,7 @@ final class GaugeTests: XCTestCase {
my_gauge 0.05
"""))
}
#endif

func testGaugeStandalone() {
let gauge = prom.createGauge(forType: Int.self, named: "my_gauge", helpText: "Gauge for testing", initialValue: 10, withLabelType: BaseLabels.self)
Expand Down
4 changes: 3 additions & 1 deletion Tests/SwiftPrometheusTests/HistogramTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@ final class HistogramTests: XCTestCase {
my_histogram_sum{myValue="labels"} 3.0\n
""")
}


#if os(Linux)
func testHistogramTime() {
let histogram = prom.createHistogram(forType: Double.self, named: "my_histogram")
let delay = 0.05
Expand All @@ -104,6 +105,7 @@ final class HistogramTests: XCTestCase {
my_histogram_sum 0.05
"""))
}
#endif

func testHistogramStandalone() {
let histogram = prom.createHistogram(forType: Double.self, named: "my_histogram", helpText: "Histogram for testing", buckets: [0.5, 1, 2, 3, 5, Double.greatestFiniteMagnitude], labels: BaseHistogramLabels.self)
Expand Down
4 changes: 3 additions & 1 deletion Tests/SwiftPrometheusTests/SummaryTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ final class SummaryTests: XCTestCase {
my_summary_sum 10010.0\n
""")
}


#if os(Linux)
func testSummaryTime() {
let summary = prom.createSummary(forType: Double.self, named: "my_summary", helpText: "Summary for testing", quantiles: [0.5, 0.9, 0.99], labels: BaseSummaryLabels.self)
let delay = 0.05
Expand All @@ -117,6 +118,7 @@ final class SummaryTests: XCTestCase {
let sections = summary.collect().split(separator: "\n").map(String.init).enumerated().map { i, s in s.starts(with: lines[i]) }
XCTAssert(sections.filter { !$0 }.isEmpty)
}
#endif

func testSummaryStandalone() {
let summary = prom.createSummary(forType: Double.self, named: "my_summary", helpText: "Summary for testing", quantiles: [0.5, 0.9, 0.99], labels: BaseSummaryLabels.self)
Expand Down
34 changes: 0 additions & 34 deletions circle.yml
View file
Open in desktop

This file was deleted.