Pending Example Impovement #491

takecian · 2016-02-23T07:00:17Z

This PR is related to #490 .

print detailed description for pending example
execute before/after hooks for pending example
rename Filter.pending to Filter.excluded

please review, thanks.
Takeshi.

…mple

takecian · 2016-03-29T07:17:35Z

@modocache, if you have spare time, it would be helpful that you review this! 👍

modocache · 2016-05-22T17:43:04Z

Sources/QuickTests/FunctionalTests/PendingTests.swift

    }

    func testBeforeEachDoesNotRunForContextsWithOnlyPendingExamples() {
        onlyPendingExamplesBeforeEachExecutedCount = 0

        qck_runSpec(FunctionalTests_PendingSpec.self)
-        XCTAssertEqual(onlyPendingExamplesBeforeEachExecutedCount, 0)
+        XCTAssertEqual(onlyPendingExamplesBeforeEachExecutedCount, 1)


This assertion no longer matches the name of the test. The test is named "test that beforeEach does not run for contexts with only pending examples", whereas this assertion verifies that beforeEach does run. The same goes for testBeforeEachOnlyRunForEnabledExamples above.

So, why do these tests exist? pending examples were first implemented in #38. At that time, Quick had an ad hoc test suite: a single test file that used Quick and was expected to pass. I added in-depth tests in #154, which included tests for pending in 6b7681b. Unfortunately, I neglected to realize the importance of documenting in the commit message whether beforeEach not running was intended, or merely the behavior at the time that I wrote these tests.

I think these tests merely documented the behavior of pending at the time. It seems like the purpose of this PR is to ensure that beforeEach, afterEach, beforeSuite, and afterSuite always run, even for pending examples. If that's the case, please:

Delete the testBeforeEachOnlyRunForEnabledExamples and testBeforeEachDoesNotRunForContextsWithOnlyPendingExamples tests.

Add tests for beforeEach, afterEach, beforeSuite, and afterSuite.

I updated test(add tests for beforeEach, afterEach, beforeSuite, and afterSuite) and create new target.
See also, #491 (comment) 😄

modocache · 2016-05-22T17:43:36Z

@takecian This is awesome!! Thanks for working on this, and sorry for the wait in reviewing.

modocache · 2016-05-22T17:44:31Z

Sources/Quick/DSL/World+DSL.swift

    }
-
+    


nit-pick: Remove this extra whitespace.

modocache · 2016-05-22T18:02:25Z

This looks great! I think the tests need some additional work, though. Thanks, @takecian!

takecian · 2016-05-23T03:17:52Z

Thank you for review! I'll fix it!

takecian · 2016-05-23T06:34:41Z

Hi @modocache , I've updated comment and added test for pending.
It would be helpful to review this.

Thanks!

modocache · 2016-05-23T11:27:44Z

Sources/QuickTests/FunctionalTests/PendingTests.swift

+            beforeEach { isBeforeEachCalled = true }
+            pending("an example that will not run") {}
+            afterEach { isAfterEachCalled = true }
+            afterSuite { isAfterSuiteCalled = true }


So, this is almost what we want. However, the beforeSuite and afterSuite tests here will pass even without your changes. This is because this test suite contains non-pending examples. To really test these, you'd need an entirely new test target in the Quick workspace -- just like the QuickFunctionalTests target.

I think we should test this in a new QuickPendingTests target.

Oh! I understand. I will create new test target and move this test into it.

…ases into QuickPendingTests.

takecian · 2016-05-23T14:29:28Z

I've created new test target for "pending" and merged master branch.
How do you feel?

modocache · 2016-05-23T14:33:27Z

Looking good! There are build failures on Linux, though. I think you'll need to use #file and #line instead of __FILE__ and __LINE__.

modocache · 2016-05-23T14:34:48Z

Documentation/en-us/TestUsingTestDoubles.md

+
+Mock makes us easy to verify behavior which works with other objects.
+
+For more details about writing test, see https://realm.io/news/testing-in-swift/ .


I am planning on reviewing this additional documentation in #487, but I don't think you meant to include it in this pull request. Could you take it out?

takecian · 2016-05-24T03:02:30Z

I did,

Remove markdown file change of Add a document about mock #487
Fix Nimble version
Remove extra space
Restore pending test suite for QuickFunctionalTest.

However build error on linux still occcurs. I will work for it.

takecian · 2016-05-25T15:11:01Z

@modocache , I've fixed errors on linux.
Could you review again?

karagraysen · 2016-08-19T22:15:17Z

@modocache: Do you have time to review this?

karagraysen · 2016-08-23T19:46:00Z

@modocache: Re-pinging, just in case the notification or email got lost. 👍

modocache · 2016-08-23T20:04:05Z

Didn't get lost, but it's a lot to review! 😅

I'll need to find some time to read through it. Hopefully sometime this week.

karagraysen · 2016-08-23T20:04:53Z

Okay, no problem. Didn't know if you had seen it or not. 😸 I'll assign the PR to you?

karagraysen · 2016-12-12T19:02:35Z

Bringing attention back to this. Could we get the merge conflicts fixed @takecian and have it reviewed @modocache. Cheers! :)

karagraysen · 2016-12-31T14:54:10Z

@takecian: Please fix the merge conflicts so we can finish reviewing your changes.

ikesyo · 2020-10-08T15:51:17Z

Closing as this is stale/abandoned.

Fujiki Takeshi added 5 commits February 23, 2016 15:38

change filter name "pending" to "excluded"

f8debd7

add Filter "pending"

131d7f6

execute before/after for pending example, print detail of pending exa…

af8518a

…mple

update test for pending example

a3c495b

add default value for pending argument

b54c667

takecian mentioned this pull request Mar 11, 2016

Using pending causes custom Configuration hooks not to run #490

Open

modocache reviewed May 22, 2016
View reviewed changes

Sources/Quick/DSL/World+DSL.swift Outdated

}

Copy link

Member

modocache May 22, 2016

Choose a reason for hiding this comment

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

nit-pick: Remove this extra whitespace.

Fujiki Takeshi added 2 commits May 23, 2016 12:05

Remvoe extra whitespace

0a7dc9c

rephrase documentation for pending, excluded

7c73b01

Fujiki Takeshi added 2 commits May 23, 2016 12:21

add __unused attribute for closure parameters of pending

d7f7bd6

Update test for pending functionality

b69447e

modocache reviewed May 23, 2016
View reviewed changes

takecian added 2 commits May 23, 2016 22:44

create TestTarget(QuickPendingTests) for pending. Move pending test c…

bad9967

…ases into QuickPendingTests.

Merge origin master

c5ebc72

modocache reviewed May 23, 2016
View reviewed changes

takecian and others added 3 commits May 24, 2016 10:24

Remove extra space

a5953f4

Update Nimble version

b597695

Add pending test suite into linux test cases

d6e9ccd

takecian added 2 commits May 25, 2016 23:32

Add QuickPendingTest dependency into Package.swift

230cba0

Add main file for PendingTest

549b051

karagraysen added the github-review-needs-review label Aug 10, 2016

karagraysen self-assigned this Aug 10, 2016

karagraysen changed the title ~~pending example impovement~~ Pending Example Impovement Aug 11, 2016

takecian added 4 commits August 19, 2016 12:05

Merge branch 'master' into issue490

ff003a1

Remove documentation change

e70ff46

Remove documentation change

c519088

Remove documentation change

9e68ca7

karagraysen assigned modocache and unassigned karagraysen Aug 23, 2016

karagraysen self-assigned this Nov 8, 2016

ikesyo mentioned this pull request Feb 3, 2019

Unique identifiers for all test examples #771

Closed

takecian unassigned karagraysen Mar 3, 2019

ikesyo linked an issue Oct 8, 2020 that may be closed by this pull request

Using pending causes custom Configuration hooks not to run #490

Open

ikesyo closed this Oct 8, 2020

ikesyo unassigned modocache Oct 8, 2020

briancroom mentioned this pull request May 21, 2016

[SR-1541] List XCTest test methods from swiftpm swiftlang/swift-package-manager#5287

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pending Example Impovement #491

Pending Example Impovement #491

takecian commented Feb 23, 2016

takecian commented Mar 29, 2016

modocache May 22, 2016

takecian Aug 19, 2016

modocache commented May 22, 2016

modocache May 22, 2016

modocache commented May 22, 2016

takecian commented May 23, 2016

takecian commented May 23, 2016

modocache May 23, 2016

takecian May 23, 2016

takecian commented May 23, 2016

modocache commented May 23, 2016

modocache May 23, 2016

takecian commented May 24, 2016 •

edited

Loading

takecian commented May 25, 2016

karagraysen commented Aug 19, 2016

karagraysen commented Aug 23, 2016

modocache commented Aug 23, 2016

karagraysen commented Aug 23, 2016

karagraysen commented Dec 12, 2016

karagraysen commented Dec 31, 2016

ikesyo commented Oct 8, 2020


		Mock makes us easy to verify behavior which works with other objects.

		For more details about writing test, see https://realm.io/news/testing-in-swift/ .

Pending Example Impovement #491

Pending Example Impovement #491

Conversation

takecian commented Feb 23, 2016

takecian commented Mar 29, 2016

modocache May 22, 2016

Choose a reason for hiding this comment

takecian Aug 19, 2016

Choose a reason for hiding this comment

modocache commented May 22, 2016

modocache May 22, 2016

Choose a reason for hiding this comment

modocache commented May 22, 2016

takecian commented May 23, 2016

takecian commented May 23, 2016

modocache May 23, 2016

Choose a reason for hiding this comment

takecian May 23, 2016

Choose a reason for hiding this comment

takecian commented May 23, 2016

modocache commented May 23, 2016

modocache May 23, 2016

Choose a reason for hiding this comment

takecian commented May 24, 2016 • edited Loading

takecian commented May 25, 2016

karagraysen commented Aug 19, 2016

karagraysen commented Aug 23, 2016

modocache commented Aug 23, 2016

karagraysen commented Aug 23, 2016

karagraysen commented Dec 12, 2016

karagraysen commented Dec 31, 2016

ikesyo commented Oct 8, 2020

takecian commented May 24, 2016 •

edited

Loading