8000 Fix empty 'Selected tests' suites by VojtaStavik · Pull Request #901 · Quick/Quick · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix empty 'Selected tests' suites #901

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Sep 11, 2019
Merged

Fix empty 'Selected tests' suites #901

merged 6 commits into from
Sep 11, 2019

Conversation

VojtaStavik
Copy link
Contributor

This PR fixes the regression introduced in a3ece4f. Closes #891

The defaultTestSuite method wasn't called when the 'Selected tests' type of test suite was executed. This caused the spec method not being invoked and the examples for the given class were never built.

This PR changes the behaviour and introduces a new buildExamplesIfNeeded method. This method can be called safely multiple times. We call it from both the defaultTestSuite and testInvocations methods. This ensures the examples are built properly no matter which method is called (or in which order).

Checklist - While not every PR needs it, new features should consider this list:

  • Does this have tests?
  • Does this have documentation?
  • Does this break the public API (Requires major version bump)?
  • Is this a new feature (Requires minor version bump)?

@QuickBot
Copy link
1 Warning
⚠️ Big PR

Generated by 🚫 Danger

@VojtaStavik VojtaStavik force-pushed the fix-selected-tests branch 2 times, most recently from c86cf20 to c74bc8d Compare July 18, 2019 14:10
@VojtaStavik
Add a failing test for 'Selected tests' invocation
04e9561 
This is a failing test for Quick#891
@VojtaStavik
Build examples also when testInvocations is invoked
a1da480 
When `Selected tests` suite is executed, XCTest doesn't call `defaultTestSuite`. The examples are now built lazily when needed.
@trinhngocthuyen trinhngocthuyen mentioned this pull request Aug 20, 2019
Closed
ikesyo
ikesyo approved these changes Sep 11, 2019
View reviewed changes
Copy link
Member
@ikesyo ikesyo left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for fixing this! 👍

@ikesyo ikesyo merged commit 1519828 into Quick:master Sep 11, 2019
This was referenced Sep 11, 2019
Closed
Closed
8000
@ikesyo
Copy link
Member
ikesyo commented Sep 11, 2019

It seems that by this fix, a crash occurs when using focused tests 😕

2019-09-12 08:09:24.170519+0900 xctest[11754:14170830] *** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'An exception occurred when building Quick's example groups.
Some possible reasons this might happen include:

- An 'expect(...).to' expectation was evaluated outside of an 'it', 'context', or 'describe' block
- 'sharedExamples' was called twice with the same name
- 'itBehavesLike' was called with a name that is not registered as a shared example

Here's the original exception: 'NSInternalInconsistencyException', reason: 'A shared example named 'combineLatest examples' has already been registered.', userInfo: '(null)''
*** First throw call stack:
(
	0   CoreFoundation                      0x00007fff3d62d2fd __exceptionPreprocess + 256
	1   libobjc.A.dylib                     0x00007fff67d0ba17 objc_exception_throw + 48
	2   CoreFoundation                      0x00007fff3d62d12f +[NSException raise:format:] + 201
	3   Quick                               0x0000000103642857 __34+[QuickSpec buildExamplesIfNeeded]_block_invo
libc++abi.dylib: terminating with uncaught exception of type NSException

@VojtaStavik
Copy link
Contributor Author

This crash looks familiar. Let me double check again. it’s been a while :)

ikesyo
ikesyo reviewed Sep 12, 2019
View reviewed changes
Sources/QuickObjectiveC/QuickSpec.m
[QuickConfiguration class];
World *world = [World sharedWorld];

NSArray *examples = [world examplesForSpecClass:[self class]];
Copy link
Member
@ikesyo ikesyo Sep 12, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

This should be rootExampleGroupForSpecClass. When in focued tests, the return value for a class whose examples are not focused entirely is empty.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice! I'll open a new PR with the fix shortly.

@VojtaStavik
Copy link
Contributor Author

@ikesyo What's your way to reproduce the crash? I can't replicate it.

@ikesyo
Copy link
Member
ikesyo commented Sep 12, 2019 •
edited
Loading

The crash happens when a shared example is registered in a unfocused spec class. In that case,

Quick/Sources/QuickObjectiveC/QuickSpec.m

Lines 94 to 111 in da6761d

[world performWithCurrentExampleGroup:rootExampleGroup closure:^{
QuickSpec *spec = [self new];
@try {
[spec spec];
}
@catch (NSException *exception) {
[NSException raise:NSInternalInconsistencyException
format:@"An exception occurred when building Quick's example groups.\n"
@"Some possible reasons this might happen include:\n\n"
@"- An 'expect(...).to' expectation was evaluated outside of "
@"an 'it', 'context', or 'describe' block\n"
@"- 'sharedExamples' was called twice with the same name\n"
@"- 'itBehavesLike' was called with a name that is not registered as a shared example\n\n"
@"Here's the original exception: '%@', reason: '%@', userInfo: '%@'",
exception.name, exception.reason, exception.userInfo];
}
}];
when the code block above is executed multiple times, that shared example will be registered multiple times thus an error will be thrown (I found the crash in ReactiveSwift repository).

Quick/Sources/Quick/World.swift

Lines 236 to 240 in d3f39da

private func raiseIfSharedExampleAlreadyRegistered(_ name: String) {
if sharedExamples[name] != nil {
raiseError("A shared example named '\(name)' has already been registered.")
}
}

This case was not covered in QuickFocusedTests unfortunately. 😭

@ikesyo
Copy link
Member
ikesyo commented Sep 12, 2019

So adding sharedExamples("foo") { _ in } to _FunctionalTests_FocusedSpec_Unfocused.spec() will make it crash.

@VojtaStavik
Copy link
Contributor Author

Cool, thank you! I'll put the test for it + the fix into a new PR. Thanks for your help @ikesyo.

VojtaStavik added a commit to industrialbinaries/Quick that referenced this pull request Sep 13, 2019
@VojtaStavik
Add failing test
68437a8 
This is a failing test for the crash reported here: Quick#901 (comment)
@VojtaStavik VojtaStavik mentioned this pull request Sep 13, 2019
Closed
@ikesyo ikesyo mentioned this pull request Sep 14, 2019
Merged
VojtaStavik added a commit to industrialbinaries/Quick that referenced this pull request Sep 16, 2019
@VojtaStavik
Build examples only for classes without root example group
49d06e7 
This commit fixes the crash reported in Quick#901 (comment)
@VojtaStavik VojtaStavik mentioned this pull request Sep 16, 2019
Merged
VojtaStavik added a commit to industrialbinaries/Quick that referenced this pull request Sep 16, 2019
@VojtaStavik
Build examples only for classes without root example group
279ffcd 
This commit fixes the crash reported in Quick#901 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ikesyo ikesyo ikesyo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Trying to run a specific spec does not run any tests
3 participants
@VojtaStavik @QuickBot @ikesyo
0