Skip to content
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

Fix bug where expect(nil).toAlways(equal(0)) would erroneously pass #1121

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
31 changes: 21 additions & 10 deletions Sources/Nimble/Polling+AsyncAwait.swift
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,19 @@ internal actor Poller<T> {
file: expression.location.file,
line: expression.location.line,
fnName: fnName) {
self.updateMatcherResult(result: try await matcherRunner())
.toBoolean(expectation: style)
if self.updateMatcherResult(result: try await matcherRunner())
.toBoolean(expectation: style) {
if matchStyle.isContinous {
return .incomplete
}
return .finished(true)
} else {
if matchStyle.isContinous {
return .finished(false)
} else {
return .incomplete
}
}
}
return processPollResult(result.toPollResult(), matchStyle: matchStyle, lastMatcherResult: lastMatcherResult, fnName: fnName)
}
Expand Down Expand Up @@ -152,7 +163,7 @@ extension SyncExpectation {
description: description) {
await poll(
expression: asyncExpression,
style: .toMatch,
style: .toNotMatch,
matchStyle: .never,
timeout: until,
poll: pollInterval,
Expand Down Expand Up @@ -186,7 +197,7 @@ extension SyncExpectation {
description: description) {
await poll(
expression: asyncExpression,
style: .toNotMatch,
style: .toMatch,
matchStyle: .always,
timeout: until,
poll: pollInterval,
Expand Down Expand Up @@ -282,7 +293,7 @@ extension SyncExpectation {
description: description) {
await poll(
expression: asyncExpression,
style: .toMatch,
style: .toNotMatch,
matchStyle: .never,
timeout: until,
poll: pollInterval,
Expand Down Expand Up @@ -316,7 +327,7 @@ extension SyncExpectation {
description: description) {
await poll(
expression: asyncExpression,
style: .toNotMatch,
style: .toMatch,
matchStyle: .always,
timeout: until,
poll: pollInterval,
Expand Down Expand Up @@ -409,7 +420,7 @@ extension AsyncExpectation {
description: description) {
await poll(
expression: expression,
style: .toMatch,
style: .toNotMatch,
matchStyle: .never,
timeout: until,
poll: pollInterval,
Expand Down Expand Up @@ -442,7 +453,7 @@ extension AsyncExpectation {
description: description) {
await poll(
expression: expression,
style: .toNotMatch,
style: .toMatch,
matchStyle: .always,
timeout: until,
poll: pollInterval,
Expand Down Expand Up @@ -533,7 +544,7 @@ extension AsyncExpectation {
description: description) {
await poll(
expression: expression,
style: .toMatch,
style: .toNotMatch,
matchStyle: .never,
timeout: until,
poll: pollInterval,
Expand Down Expand Up @@ -566,7 +577,7 @@ extension AsyncExpectation {
description: description) {
await poll(
expression: expression,
style: .toNotMatch,
style: .toMatch,
matchStyle: .always,
timeout: until,
poll: pollInterval,
Expand Down
20 changes: 10 additions & 10 deletions Sources/Nimble/Polling+Require.swift
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ extension SyncRequirement {
expression,
.toNotMatch,
poll(
style: .toMatch,
style: .toNotMatch,
matchStyle: .never,
matcher: matcher,
timeout: until,
Expand Down Expand Up @@ -158,7 +158,7 @@ extension SyncRequirement {
expression,
.toMatch,
poll(
style: .toNotMatch,
style: .toMatch,
matchStyle: .always,
matcher: matcher,
timeout: until,
Expand Down Expand Up @@ -266,7 +266,7 @@ extension SyncRequirement {
description: description) {
await poll(
expression: asyncExpression,
style: .toMatch,
style: .toNotMatch,
matchStyle: .never,
timeout: until,
poll: pollInterval,
Expand Down Expand Up @@ -300,7 +300,7 @@ extension SyncRequirement {
description: description) {
await poll(
expression: asyncExpression,
style: .toNotMatch,
style: .toMatch,
matchStyle: .always,
timeout: until,
poll: pollInterval,
Expand Down Expand Up @@ -396,7 +396,7 @@ extension SyncRequirement {
description: description) {
await poll(
expression: asyncExpression,
style: .toMatch,
style: .toNotMatch,
matchStyle: .never,
timeout: until,
poll: pollInterval,
Expand Down Expand Up @@ -430,7 +430,7 @@ extension SyncRequirement {
description: description) {
await poll(
expression: asyncExpression,
style: .toNotMatch,
style: .toMatch,
matchStyle: .always,
timeout: until,
poll: pollInterval,
Expand Down Expand Up @@ -523,7 +523,7 @@ extension AsyncRequirement {
description: description) {
await poll(
expression: expression,
style: .toMatch,
style: .toNotMatch,
matchStyle: .never,
timeout: until,
poll: pollInterval,
Expand Down Expand Up @@ -556,7 +556,7 @@ extension AsyncRequirement {
description: description) {
await poll(
expression: expression,
style: .toNotMatch,
style: .toMatch,
matchStyle: .always,
timeout: until,
poll: pollInterval,
Expand Down Expand Up @@ -647,7 +647,7 @@ extension AsyncRequirement {
description: description) {
await poll(
expression: expression,
style: .toMatch,
style: .toNotMatch,
matchStyle: .never,
timeout: until,
poll: pollInterval,
Expand Down Expand Up @@ -680,7 +680,7 @@ extension AsyncRequirement {
description: description) {
await poll(
expression: expression,
style: .toNotMatch,
style: .toMatch,
matchStyle: .always,
timeout: until,
poll: pollInterval,
Expand Down
26 changes: 23 additions & 3 deletions Sources/Nimble/Polling.swift
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,15 @@ public struct PollingDefaults {

internal enum AsyncMatchStyle {
case eventually, never, always

var isContinous: Bool {
switch self {
case .eventually:
return false
case .never, .always:
return true
}
}
}

// swiftlint:disable:next function_parameter_count
Expand All @@ -63,7 +72,18 @@ internal func poll<T>(
line: actualExpression.location.line,
fnName: fnName) {
lastMatcherResult = try matcher.satisfies(uncachedExpression)
return lastMatcherResult!.toBoolean(expectation: style)
if lastMatcherResult!.toBoolean(expectation: style) {
if matchStyle.isContinous {
return .incomplete
}
return .finished(true)
} else {
if matchStyle.isContinous {
return .finished(false)
} else {
return .incomplete
}
}
}
return processPollResult(result, matchStyle: matchStyle, lastMatcherResult: lastMatcherResult, fnName: fnName)
}
Expand Down Expand Up @@ -220,7 +240,7 @@ extension SyncExpectation {
expression,
.toNotMatch,
poll(
style: .toMatch,
style: .toNotMatch,
matchStyle: .never,
matcher: matcher,
timeout: until,
Expand Down Expand Up @@ -271,7 +291,7 @@ extension SyncExpectation {
expression,
.toMatch,
poll(
style: .toNotMatch,
style: .toMatch,
matchStyle: .always,
matcher: matcher,
timeout: until,
Expand Down
10 changes: 5 additions & 5 deletions Sources/Nimble/Utils/AsyncAwait.swift
Original file line number Diff line number Diff line change
Expand Up @@ -165,11 +165,11 @@ private func timeout<T>(timeoutQueue: DispatchQueue, timeoutInterval: NimbleTime
return await promise.value
}

private func poll(_ pollInterval: NimbleTimeInterval, expression: @escaping () async throws -> Bool) async -> AsyncPollResult<Bool> {
private func poll(_ pollInterval: NimbleTimeInterval, expression: @escaping () async throws -> PollStatus) async -> AsyncPollResult<Bool> {
for try await _ in AsyncTimerSequence(interval: pollInterval) {
do {
if try await expression() {
return .completed(true)
if case .finished(let result) = try await expression() {
return .completed(result)
}
} catch {
return .errorThrown(error)
Expand Down Expand Up @@ -199,7 +199,7 @@ private func runPoller(
pollInterval: NimbleTimeInterval,
awaiter: Awaiter,
fnName: String = #function, file: FileString = #file, line: UInt = #line,
expression: @escaping () async throws -> Bool
expression: @escaping () async throws -> PollStatus
) async -> AsyncPollResult<Bool> {
awaiter.waitLock.acquireWaitingLock(
fnName,
Expand Down Expand Up @@ -324,7 +324,7 @@ internal func pollBlock(
file: FileString,
line: UInt,
fnName: String = #function,
expression: @escaping () async throws -> Bool) async -> AsyncPollResult<Bool> {
expression: @escaping () async throws -> PollStatus) async -> AsyncPollResult<Bool> {
await runPoller(
timeoutInterval: timeoutInterval,
pollInterval: pollInterval,
Expand Down
11 changes: 8 additions & 3 deletions Sources/Nimble/Utils/PollAwait.swift
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,11 @@
}
}

internal enum PollStatus {
case finished(Bool)
case incomplete
}

/// Holds the resulting value from an asynchronous expectation.
/// This class is thread-safe at receiving a "response" to this promise.
internal final class AwaitPromise<T> {
Expand Down Expand Up @@ -155,7 +160,7 @@
self.trigger = trigger
}

func timeout(_ timeoutInterval: NimbleTimeInterval, forcefullyAbortTimeout: NimbleTimeInterval) -> Self {

Check warning on line 163 in Sources/Nimble/Utils/PollAwait.swift

View workflow job for this annotation

GitHub Actions / lint

Function Body Length Violation: Function body should span 50 lines or less excluding comments and whitespace: currently spans 54 lines (function_body_length)
/// = Discussion =
///
/// There's a lot of technical decisions here that is useful to elaborate on. This is
Expand Down Expand Up @@ -399,11 +404,11 @@
file: FileString,
line: UInt,
fnName: String = #function,
expression: @escaping () throws -> Bool) -> PollResult<Bool> {
expression: @escaping () throws -> PollStatus) -> PollResult<Bool> {
let awaiter = NimbleEnvironment.activeInstance.awaiter
let result = awaiter.poll(pollInterval) { () throws -> Bool? in
if try expression() {
return true
if case .finished(let result) = try expression() {
return result
}
return nil
}
Expand All @@ -413,4 +418,4 @@
return result
}

#endif // #if !os(WASI)

Check warning on line 421 in Sources/Nimble/Utils/PollAwait.swift

View workflow job for this annotation

GitHub Actions / lint

File Length Violation: File should contain 400 lines or less: currently contains 421 (file_length)
6 changes: 6 additions & 0 deletions Tests/NimbleTests/AsyncAwaitTest+Require.swift
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,9 @@ final class AsyncAwaitRequireTest: XCTestCase { // swiftlint:disable:this type_b
await failsWithErrorMessage("unexpected error thrown: <\(errorToThrow)>") {
try await require { try self.doThrowError() }.neverTo(equal(0))
}
await failsWithErrorMessage("expected to never equal <1>, got <1>") {
try await require(1).toNever(equal(1))
}
}

func testToAlwaysPositiveMatches() async throws {
Expand Down Expand Up @@ -276,6 +279,9 @@ final class AsyncAwaitRequireTest: XCTestCase { // swiftlint:disable:this type_b
await failsWithErrorMessage("unexpected error thrown: <\(errorToThrow)>") {
try await require { try self.doThrowError() }.alwaysTo(equal(0))
}
await failsWithErrorMessage("expected to always equal <0>, got <nil> (use beNil() to match nils)") {
try await require(nil).toAlways(equal(0))
}
}
}

Expand Down
3 changes: 3 additions & 0 deletions Tests/NimbleTests/AsyncAwaitTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,9 @@ final class AsyncAwaitTest: XCTestCase { // swiftlint:disable:this type_body_len
await failsWithErrorMessage("unexpected error thrown: <\(errorToThrow)>") {
await expect { try self.doThrowError() }.alwaysTo(equal(0))
}
await failsWithErrorMessage("expected to always equal <0>, got <nil> (use beNil() to match nils)") {
await expect(nil).toAlways(equal(0))
}
}
}

Expand Down
6 changes: 6 additions & 0 deletions Tests/NimbleTests/PollingTest+Require.swift
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,9 @@ final class PollingRequireTest: XCTestCase {
failsWithErrorMessage("unexpected error thrown: <\(errorToThrow)>") {
try require { try self.doThrowError() }.neverTo(equal(0))
}
failsWithErrorMessage("expected to never equal <1>, got <1>") {
try require(1).toNever(equal(1))
}
}

func testToAlwaysPositiveMatches() throws {
Expand Down Expand Up @@ -207,6 +210,9 @@ final class PollingRequireTest: XCTestCase {
failsWithErrorMessage("unexpected error thrown: <\(errorToThrow)>") {
try require { try self.doThrowError() }.alwaysTo(equal(0))
}
failsWithErrorMessage("expected to always equal <1>, got <nil> (use beNil() to match nils)") {
try require(nil).toAlways(equal(1))
}
}
}

Expand Down
6 changes: 6 additions & 0 deletions Tests/NimbleTests/PollingTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,9 @@ final class PollingTest: XCTestCase {
failsWithErrorMessage("unexpected error thrown: <\(errorToThrow)>") {
expect { try self.doThrowError() }.neverTo(equal(0))
}
failsWithErrorMessage("expected to never equal <0>, got <0>") {
expect(0).toNever(equal(0))
}
}

func testToAlwaysPositiveMatches() {
Expand Down Expand Up @@ -328,6 +331,9 @@ final class PollingTest: XCTestCase {
failsWithErrorMessage("unexpected error thrown: <\(errorToThrow)>") {
expect { try self.doThrowError() }.alwaysTo(equal(0))
}
failsWithErrorMessage("expected to always equal <0>, got <nil> (use beNil() to match nils)") {
expect(nil).toAlways(equal(0))
}
}
}

Expand Down
Loading