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

[SR-14597] Misleading documentation for XCTestExpectation.expectedFulfillmentCount #346

Open
swift-ci opened this issue May 5, 2021 · 5 comments

Comments

@swift-ci
Copy link

swift-ci commented May 5, 2021

Previous ID SR-14597
Radar rdar://problem/77673444
Original Reporter arhaiziya (JIRA User)
Type Bug

Attachment: Download

Environment

Xcode: 12.4 (12D4e)
Swift 5.3.2 (swiftlang-1200.0.45 clang-1200.0.32.28)

Additional Detail from JIRA
Votes 0
Component/s XCTest
Labels Bug, Documentation, StarterBug
Assignee jiaren wang (JIRA)
Priority Medium

md5: 774aff3193998de82f1751fe0c87c0b9

Issue Description:

Documentation for expectedfulfillmentcount has the following note:

The value of expectedFulfillmentCount is ignored when isInverted is true.

However, the following snippet shows different behavior. The first test succeeded and the second one did not (see attachments for test results).

import XCTest

class SomeTestCase: XCTestCase {

    func testExpectationFulfilled() {
        let expectation = XCTestExpectation()
        expectation.expectedFulfillmentCount = 3
        expectation.isInverted = true
        expectation.fulfill()
        expectation.fulfill()
        wait(for: [expectation], timeout: 1.0)
    }
 
    func testExpectationNotFulfilled() {
        let expectation = XCTestExpectation()
        expectation.isInverted = true
        expectation.expectedFulfillmentCount = 3
        expectation.fulfill()
        expectation.fulfill()
        expectation.fulfill()
        wait(for: [expectation], timeout: 1.0)
    }

}

Looks like a bug in docs or the behavior, but expectedFulfillmentCount is surely not ignored and is used as a threshold value.

@swift-ci
Copy link
Author

swift-ci commented May 6, 2021

Comment by jiaren wang (JIRA)

arhaiziya (JIRA User) Here is what I am found: Fulfilled inverse expectations would be catch.

The note for Documentation for expectedfulfillmentcount has a precondition: Expectation is not fulfilled before wait was called.

if expectation.isFulfilled {
       // Check for any fulfilled inverse expectations. If they were fulfilled before wait was called,
       // this is where we'd catch that.
       if expectation.isInverted {
             return .fulfilledInvertedExpectation(invertedExpectation: expectation)
       } else {
             fulfilledExpectations.append(expectation)
       }
} else {
     unfulfilledExpectations.append(expectation)
}

@typesanitizer
Copy link

@swift-ci create

@swift-ci
Copy link
Author

Comment by Uladzislau Tarasevich (JIRA)

jiaren wang (JIRA User) Unfortunately, I'm barely getting what you mean.

Yes, fulfilled inverse expectations would be caught. But looking at the note in the docs and some code comments, it looks like that the expectedfulfillmentcount should be ignored (be equal to 1) for inverted expectations, isn't it?

/// The number of times `fulfill()` must be called on the expectation in order for it
/// to report complete fulfillment to its waiter. Default is 1.
/// This value must be greater than 0 and is not meaningful if combined with `isInverted`.
    open var expectedFulfillmentCount: Int {

So I believe that according to the docs setting expectedFulfillmentCount should be ignored for inverted expectations and the expectation should be fulfilled at the first call of fulfill(), regardless of any expectedFulfillmentCount value. That's what I am trying to say.

Possible problem place:

private func queue_fulfill(sourceLocation: SourceLocation) -> Bool {
    dispatchPrecondition(condition: .onQueue(XCTWaiter.subsystemQueue))

    numberOfFulfillments += 1

    if numberOfFulfillments == queue_expectedFulfillmentCount { // <<<<<
        queue_isFulfilled = true
        _fulfillmentSourceLocation = sourceLocation
        queue_fulfillmentToken = XCTestExpectation.queue_nextMonotonicallyIncreasingToken()
        return true
    } else {
        return false
    }
}

P.S.

  1. If this is actually a bug, I would love to make a contribution to the repo myself, if it's possible of course 🙂
  2. Sorry for the late response. I thought there would be some kind of notification by e-mail.

@swift-ci
Copy link
Author

Comment by jiaren wang (JIRA)

arhaiziya (JIRA User) Thanks for your response.
According to your reply, It looks like the condition that you point out is missing isInverted checked.
Maybe the following is the solution?

    private func queue_fulfill(sourceLocation: SourceLocation) -> Bool {
        dispatchPrecondition(condition: .onQueue(XCTWaiter.subsystemQueue))

        numberOfFulfillments += 1

        if numberOfFulfillments == queue_expectedFulfillmentCount || queue_isInverted {
            queue_isFulfilled = true
            _fulfillmentSourceLocation = sourceLocation
            queue_fulfillmentToken = XCTestExpectation.queue_nextMonotonicallyIncreasingToken()
            return true
        } else {
            return false
        }
    }

@swift-ci
Copy link
Author

Comment by Uladzislau Tarasevich (JIRA)

jiaren wang (JIRA User) Sure, at first glance, it can be a solution. But only if that note for expectedfulfillmentcount is relevant. Either it is a problem with documentation or code.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@shahmishal shahmishal transferred this issue from swiftlang/swift May 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants