Skip to content

Make 'values' on TestCounter, TestRecorder and TestMeter public #129

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 2 commits into from
May 26, 2023

Conversation

hamzahrmalik
Copy link
Contributor

Make 'values' on TestCounter, TestRecorder and TestMeter public

Motivation:

TestTimer already has a public values: [Double]. We should also expose equivalent properties (with equivalent types) on TestCounter, TestRecorder and TestMeter

Modifications:

Change the existing values property to have a type of [Int64] or [Double] to be equivalent to the existing TestTimer.values which is [Double]. Then, make these values properties public

Result:

Users can run assertions such as values.count == 2 to ensure the expected number of values were seen. Otherwise, they are limited to checking the lastValue only

@hamzahrmalik hamzahrmalik force-pushed the expose_test_metirc_values branch from bf9d9a0 to 9a1d7c8 Compare May 26, 2023 11:53
@ktoso ktoso added this to the 2.4.0 milestone May 26, 2023
@hamzahrmalik hamzahrmalik force-pushed the expose_test_metirc_values branch from 9a1d7c8 to e9df26c Compare May 26, 2023 12:04
@hamzahrmalik hamzahrmalik marked this pull request as ready for review May 26, 2023 12:04
return Double(value)
}
return Double(value) / Double(displayUnit.scaleFromNanoseconds)
public func retrieveValueInPreferredUnit(atIndex i: Int) -> Double {
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if just valueInPreferredUnit would be fine, while making this public?

Copy link
Member

@ktoso ktoso left a comment

Choose a reason for hiding this comment

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

Yeah I think this is okey and in-line with the goal of the testkit 👍

One naming idea, wdyt?

@ktoso ktoso merged commit 3402510 into apple:main May 26, 2023
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