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

Limit number of test classes in file to one #1779

Closed
ornithocoder opened this Issue Aug 16, 2017 · 6 comments

Comments

Projects
None yet
2 participants
@ornithocoder
Contributor

ornithocoder commented Aug 16, 2017

One of the things I like the most about Quick is the possibility to isolate scenarios in nested blocks to better describe the behavior of the system under test.

As result, rare are the cases were one would need to implement two or more QuickSpecs in the same test file. A single QuickSpec per file is enough.

rule request, single quick spec per file, opt-in

ornithocoder added a commit to ornithocoder/personal-fork-swiftlint that referenced this issue Aug 16, 2017

Add quick_single_spec opt-in rule
Implements realm#1779 (Limit number of QuickSpecs in file to one).
@marcelofabri

This comment has been minimized.

Show comment
Hide comment
@marcelofabri

marcelofabri Aug 16, 2017

Collaborator

Should this also be applied to XCTestCase?

Collaborator

marcelofabri commented Aug 16, 2017

Should this also be applied to XCTestCase?

@ornithocoder

This comment has been minimized.

Show comment
Hide comment
@ornithocoder

ornithocoder Aug 16, 2017

Contributor

@marcelofabri I'm not sure, but I more inclined to say no.

One might want to test the system under test with different parameters and a single pair of setUp and tearDown might not be enough to cover all the cases. Several XCTestCase classes might be needed to define all these different "contexts".

Using Quick, these are different setups would live inside the same QuickSpec class, but nested under describes or contexts.

Contributor

ornithocoder commented Aug 16, 2017

@marcelofabri I'm not sure, but I more inclined to say no.

One might want to test the system under test with different parameters and a single pair of setUp and tearDown might not be enough to cover all the cases. Several XCTestCase classes might be needed to define all these different "contexts".

Using Quick, these are different setups would live inside the same QuickSpec class, but nested under describes or contexts.

@marcelofabri

This comment has been minimized.

Show comment
Hide comment
@marcelofabri

marcelofabri Aug 16, 2017

Collaborator

But then one could do the setup in each test or even split the XCTestCase into another file. I don't think I've ever seen a project that had two (or more) XCTestCase in the same file.

Collaborator

marcelofabri commented Aug 16, 2017

But then one could do the setup in each test or even split the XCTestCase into another file. I don't think I've ever seen a project that had two (or more) XCTestCase in the same file.

@ornithocoder

This comment has been minimized.

Show comment
Hide comment
@ornithocoder

ornithocoder Aug 16, 2017

Contributor

IMO setup should never happen inside the test. Not when using Quick/Cedar/etc.. at least. The 'it' block should be only for verifying the expectation.

I can either change this rule to cover XCTestCase or create a new (and similar rule) for XCTestCase.

Contributor

ornithocoder commented Aug 16, 2017

IMO setup should never happen inside the test. Not when using Quick/Cedar/etc.. at least. The 'it' block should be only for verifying the expectation.

I can either change this rule to cover XCTestCase or create a new (and similar rule) for XCTestCase.

@ornithocoder

This comment has been minimized.

Show comment
Hide comment
@ornithocoder

ornithocoder Aug 16, 2017

Contributor

PR updated :-)

Contributor

ornithocoder commented Aug 16, 2017

PR updated :-)

@ornithocoder ornithocoder changed the title from Limit number of QuickSpecs in file to one to Limit number of test classes in file to one Aug 16, 2017

ornithocoder added a commit to ornithocoder/personal-fork-swiftlint that referenced this issue Aug 16, 2017

Add quick_single_spec opt-in rule
Implements realm#1779 (Limit number of QuickSpecs in file to one).
@marcelofabri

This comment has been minimized.

Show comment
Hide comment
@marcelofabri

marcelofabri Aug 17, 2017

Collaborator

Closed in #1780

Collaborator

marcelofabri commented Aug 17, 2017

Closed in #1780

marcelofabri added a commit that referenced this issue Aug 17, 2017

Add quick_single_spec opt-in rule
Implements #1779 (Limit number of QuickSpecs in file to one).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment