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

Report reject macro failure properly #430

Closed
wants to merge 1 commit into from

Conversation

tzvist
Copy link

@tzvist tzvist commented May 29, 2020

Previously when messaging OCMReject only an exception was raised and the
failure was not associated with the line and file of declaration.

@dmaclach
Copy link
Contributor

Great minds think alike. I think my #431 is a superset of what you were trying to accomplish here.

@tzvist tzvist force-pushed the feature/report-reject-macro branch from df40c4b to 73830a2 Compare May 30, 2020 18:01
@tzvist
Copy link
Author

tzvist commented May 30, 2020

Great minds think alike. I think my #431 is a superset of what you were trying to accomplish here.

LOL we have opened very similar PR's, but I do not think it is a superset , my goal was to get a red dot in Xcode on failure of OCMRejec (similar to OCMVerif) by calling:

OCMReportFailure([self location], description);

If I understand correctly I don't think your PR will solve this issue. but maybe I so should open my PR on top of yours because you add the location property, that I can reuse.

Previously when messaging OCMReject only an exception was raised and the
failure was not associated with the line and file of declaration.
@dmaclach
Copy link
Contributor

I'm afraid I don't understand the win here. With the status quo I get given information about where the rejection actually occurred which I think is more useful to me than getting information about where I asked for the rejection to be checked.

@tzvist
Copy link
Author

tzvist commented Jun 19, 2020

I'm afraid I don't understand the win here. With the status quo I get given information about where the rejection actually occurred which I think is more useful to me than getting information about where I asked for the rejection to be checked.

I don't understand how do you get the information "where the rejection actually occurred" . For example for the following test:

- (void)testReject
{
    id mock = OCMClassMock([TestClassForMacroTesting class]);
    OCMReject([mock stringValue]);
   [mock stringValue];
}

I would like to get an error easier on line OCMReject([mock stringValue]) or
[mock stringValue] (and a red dot indication), but I get a red dot an Xcode indication in OCMInvocationExpectation.m file and the following log:

../OCMock/Source/OCMock/OCMInvocationExpectation.m:47: error: -[OCMockObjectMacroTests testReject] : failed: caught "NSInternalInconsistencyException", "stringValue: explicitly disallowed method invoked: stringValue"
(
	0   CoreFoundation                      0x00007fff23e3dcce __exceptionPreprocess + 350
	1   libobjc.A.dylib                     0x00007fff50b3b9b2 objc_exception_throw + 48
	2   CoreFoundation                      0x00007fff23e3db0c +[NSException raise:format:] + 188
	3   OCMockLibTests                      0x000000010e22f5d5 -[OCMInvocationExpectation handleInvocation:] + 245
	4   OCMockLibTests                      0x000000010e228ab0 -[OCMockObject handleInvocation:] + 1904
	5   OCMockLibTests                      0x000000010e22815d -[OCMockObject forwardInvocation:] + 61
	6   CoreFoundation                      0x00007fff23e42476 ___forwarding___ + 838
	7   CoreFoundation                      0x00007fff23e449b8 _CF_forwarding_prep_0 + 120
	8   OCMockLibTests                      0x000000010e1ee543 -[OCMockObjectMacroTests testReject] + 387
	9   CoreFoundation                      0x00007fff23e44c4c __invoking___ + 140
	10  CoreFoundation                      0x00007fff23e41e31 -[NSInvocation invoke] + 321
	11  XCTest                              0x000000010dad8037 __24-[XCTestCase invokeTest]_block_invoke_2 + 52
	12  XCTest                              0x000000010dad7fe3 __24-[XCTestCase invokeTest]_block_invoke.206 + 320
	13  XCTest                              0x000000010db32dc2 +[XCTestCase(Failures) performFailableBlock:testCase:testCaseRun:shouldInterruptTest:] + 69
	14  XCTest                              0x000000010db32cd4 -[XCTestCase(Failures) _performTurningExceptionsIntoFailuresInterruptAfterHandling:block:] + 115
	15  XCTest                              0x000000010dad79f6 -[XCTestCase invokeTest] + 1183
	16  XCTest                              0x000000010dad9329 __26-[XCTestCase performTest:]_block_invoke_2 + 43
	17  XCTest                              0x000000010db32dc2 +[XCTestCase(Failures) performFailableBlock:testCase:testCaseRun:shouldInterruptTest:] + 69
	18  XCTest                              0x000000010db32cd4 -[XCTestCase(Failures) _performTurningExceptionsIntoFailuresInterruptAfterHandling:block:] + 115
	19  XCTest                              0x000000010dad9260 __26-[XCTestCase performTest:]_block_invoke.359 + 86
	20  XCTest                              0x000000010db45a0d +[XCTContext runInContextForTestCase:block:] + 211
	21  XCTest                              0x000000010dad8b14 -[XCTestCase performTest:] + 566
	22  XCTest                              0x000000010db1f38e -[XCTest runTest] + 57
	23  XCTest                              0x000000010dad2d50 __27-[XCTestSuite performTest:]_block_invoke + 354
	24  XCTest                              0x000000010dad24a2 __59-[XCTestSuite _performProtectedSectionForTest:testSection:]_block_invoke + 24
	25  XCTest                              0x000000010db45a0d +[XCTContext runInContextForTestCase:block:] + 211
	26  XCTest                              0x000000010dad2459 -[XCTestSuite _performProtectedSectionForTest:testSection:] + 148
	27  XCTest                              0x000000010dad27be -[XCTestSuite performTest:] + 348
	28  XCTest                              0x000000010db1f38e -[XCTest runTest] + 57
	29  XCTest                              0x000000010dad2d50 __27-[XCTestSuite performTest:]_block_invoke + 354
	30  XCTest                              0x000000010dad24a2 __59-[XCTestSuite _performProtectedSectionForTest:testSection:]_block_invoke + 24
	31  XCTest                              0x000000010db45a0d +[XCTContext runInContextForTestCase:block:] + 211
	32  XCTest                              0x000000010dad2459 -[XCTestSuite _performProtectedSectionForTest:testSection:] + 148
	33  XCTest                              0x000000010dad27be -[XCTestSuite performTest:] + 348
	34  XCTest                              0x000000010db1f38e -[XCTest runTest] + 57
	35  XCTest                              0x000000010dad2d50 __27-[XCTestSuite performTest:]_block_invoke + 354
	36  XCTest                              0x000000010dad24a2 __59-[XCTestSuite _performProtectedSectionForTest:testSection:]_block_invoke + 24
	37  XCTest                              0x000000010db45a0d +[XCTContext runInContextForTestCase:block:] + 211
	38  XCTest                              0x000000010dad2459 -[XCTestSuite _performProtectedSectionForTest:testSection:] + 148
	39  XCTest                              0x000000010dad27be -[XCTestSuite performTest:] + 348
	40  XCTest                              0x000000010db1f38e -[XCTest runTest] + 57
	41  XCTest                              0x000000010db54f14 __44-[XCTTestRunSession runTestsAndReturnError:]_block_invoke + 171
	42  XCTest                              0x000000010db55001 __44-[XCTTestRunSession runTestsAndReturnError:]_block_invoke.100 + 96
	43  XCTest                              0x000000010daed746 -[XCTestObservationCenter _observeTestExecutionForBlock:] + 682
	44  XCTest                              0x000000010db54c9f -[XCTTestRunSession runTestsAndReturnError:] + 615
	45  XCTest                              0x000000010dab6744 -[XCTestDriver runTestsAndReturnError:] + 456
	46  XCTest                              0x000000010db4164c _XCTestMain + 2496
	47  xctest                              0x000000010d814bb1 main + 267
	48  libdyld.dylib                       0x00007fff519b910d start + 1
)
Test Case '-[OCMockObjectMacroTests testReject]' failed (0.042 seconds).
Test Suite 'OCMockObjectMacroTests' failed at 2020-06-19 19:07:04.865.
	 Executed 1 test, with 1 failure (1 unexpected) in 0.042 (0.042) seconds
Test Suite 'OCMockLibTests.xctest' failed at 2020-06-19 19:07:04.866.
	 Executed 1 test, with 1 failure (1 unexpected) in 0.042 (0.043) seconds
Test Suite 'Selected tests' failed at 2020-06-19 19:07:04.866.
	 Executed 1 test, with 1 failure (1 unexpected) in 0.042 (0.045) seconds
Program ended with exit code: 1

@erikdoe
Copy link
Owner

erikdoe commented Jun 22, 2020

Okay, I don't think it's ideal that you have to go up a few frames in the stack trace. However, I think it's not correct to report the failure in the line where the OCMReject() macro is used; and that is what this PR would result in, right?

@tzvist
Copy link
Author

tzvist commented Jun 23, 2020

Okay, I don't think it's ideal that you have to go up a few frames in the stack trace. However, I think it's not correct to report the failure in the line where the OCMReject() macro is used; and that is what this PR would result in, right?

If you set a break point on exception raise it will any way be caught in OCMReportFailure and user can go a few frames up. this is for the case that you do not have a break point and i don't think you loose anything.

@erikdoe
Copy link
Owner

erikdoe commented Jul 2, 2020

Well, you lose consistency, which has a negative impact on developer experience. Errors should be reported consistently where they occur. The error does not occur where you set up the reject, it occurs where the method is invoked. So, if the outcome of this PR is that the error would be reported at the OCMReject() then, I'm afraid, I won't merge it.

@tzvist
Copy link
Author

tzvist commented Jul 3, 2020

Well, you lose consistency, which has a negative impact on developer experience. Errors should be reported consistently where they occur. The error does not occur where you set up the reject, it occurs where the method is invoked. So, if the outcome of this PR is that the error would be reported at the OCMReject() then, I'm afraid, I won't merge it.

@tzvist tzvist closed this Jul 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants