Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Coverage autoexit #149

Closed
wants to merge 9 commits into
from

Conversation

Projects
None yet
2 participants

dodikk commented Jan 23, 2014

Fixed #148 Code coverage does not work with GHUNIT_AUTOEXIT
Added __gcov_flush() statement to runner completion

Owner

x2on commented Jan 24, 2014

Shouldn't __gcov_flush() only called if code coverage is active?

dodikk commented Jan 24, 2014

When it's not active the stub below will be called instead of "actual" flush :

void __attribute__((weak)) __gcov_flush(void) {
}
Owner

x2on commented Jan 24, 2014

Ok could you please add some comments to this ;)

dodikk commented Jan 24, 2014

Ok could you please add some comments to this ;)

Should I add those to the source code?

Owner

x2on commented Jan 24, 2014

Yes please on the source, so why is __gcov_flush() called and why is the empty function defined.

dodikk commented Jan 24, 2014

The fix works fine while I'm building GHUnit from source.
When it comes to a framework, it does not work as expected for some reason. I'm investigating it at the moment...

It's ok. Just my "Framework Search Path" was dirty.

@x2on x2on commented on the diff Feb 16, 2014

Classes-iOS/GHUnitIOSViewController.m
@@ -274,16 +274,66 @@ - (void)testRunnerDidCancel:(GHTestRunner *)runner {
[self setStatusText:@"Cancelled..."];
}
+
+void __attribute__((weak)) __gcov_flush(void) {
+
+ NSLog( @"stub __gcov_flush() invoked - ghunit" );
+
+// @dodikk : an stub is defined to be called when coverage flags below are not enabled.
@x2on

x2on Feb 16, 2014

Owner

Is that a todo?

@dodikk

dodikk Feb 17, 2014

No, it's not.
This is the comment you have requested before.

Ok could you please add some comments to this ;)

@x2on x2on commented on the diff Feb 16, 2014

Classes-iOS/GHUnitIOSViewController.m
- (void)testRunnerDidEnd:(GHTestRunner *)runner {
[self _setRunning:NO runner:runner];
[self setStatusText:[dataSource_ statusString:@"Tests finished. "]];
// Save defaults after test run
[self saveDefaults];
+
+ NSLog(@"checking __gcov_flush() symbol resolved");
+ // checking if |weak symbol| is available
+ if ( __gcov_flush )
+ {
+ NSLog(@"__gcov_flush() resolved. Invoking...");
+
+ // @dodikk : A workaround to fix issue #148
@x2on

x2on Feb 16, 2014

Owner

Also an todo?

@dodikk

dodikk Feb 17, 2014

I think it's my nickname makes you think this is a todo.
In my team the person who comments some piece of code has to sign it by leave his nickname.
Sorry for this kind of confusion.

FYI : i have used the approach below to add coverage files generation to gh-unit
https://github.com/leroymattingly/XCode5gcovPatch/

Owner

x2on commented Feb 16, 2014

Thanks for your changes.

Some comments:

  • Please remove the NSLog statements.
  • Why is the project file changed? I think there are no needed changes in it, or?
  • Could you please merge the current master in your branch?

dodikk commented Feb 17, 2014

Why is the project file changed? I think there are no needed changes in it, or?

I needed it to use gh-unit as a subproject as opposed to building a fat binary.

Owner

x2on commented Feb 17, 2014

Why is the project file changed? I think there are no needed changes in it, or?

I needed it to use gh-unit as a subproject as opposed to building a fat binary.

But this isn't needed for that pull request, or?

dodikk commented Feb 17, 2014

But this isn't needed for that pull request, or?

I'd like to leave the target out there. It's not convenient (IMHO) to debug the framework having separate targets for the device and the simulator.

You cannot just discard the changes of *.pbxproj since I have modified some project settings

GCC_GENERATE_TEST_COVERAGE_FILES = YES
GCC_INSTRUMENT_PROGRAM_FLOW_ARCS = YES

However, you can remove my target since I have not touched either your build scripts or your targets.

Owner

x2on commented Feb 17, 2014

Ok i tried to get MyTestable-iOS in the Example folder running with your changes but i get the following error:

Undefined symbols for architecture i386:
  "_llvm_gcda_emit_arcs", referenced from:
      ___llvm_gcov_writeout in GHUnitIOS(GHAsyncTestCase.o)
      ___llvm_gcov_writeout in GHUnitIOS(GHTest+JUnitXML.o)
      ___llvm_gcov_writeout in GHUnitIOS(GHTest.o)
      ___llvm_gcov_writeout in GHUnitIOS(GHTestGroup+JUnitXML.o)
      ___llvm_gcov_writeout in GHUnitIOS(GHTestGroup.o)
      ___llvm_gcov_writeout in GHUnitIOS(GHTesting.o)
      ___llvm_gcov_writeout in GHUnitIOS(GHTestOperation.o)
      ...
  "_llvm_gcda_emit_function", referenced from:
      ___llvm_gcov_writeout in GHUnitIOS(GHAsyncTestCase.o)
      ___llvm_gcov_writeout in GHUnitIOS(GHTest+JUnitXML.o)
      ___llvm_gcov_writeout in GHUnitIOS(GHTest.o)
      ___llvm_gcov_writeout in GHUnitIOS(GHTestGroup+JUnitXML.o)
      ___llvm_gcov_writeout in GHUnitIOS(GHTestGroup.o)
      ___llvm_gcov_writeout in GHUnitIOS(GHTesting.o)
      ___llvm_gcov_writeout in GHUnitIOS(GHTestOperation.o)
      ...
  "_llvm_gcda_end_file", referenced from:
      ___llvm_gcov_writeout in GHUnitIOS(GHAsyncTestCase.o)
      ___llvm_gcov_writeout in GHUnitIOS(GHTest+JUnitXML.o)
      ___llvm_gcov_writeout in GHUnitIOS(GHTest.o)
      ___llvm_gcov_writeout in GHUnitIOS(GHTestGroup+JUnitXML.o)
      ___llvm_gcov_writeout in GHUnitIOS(GHTestGroup.o)
      ___llvm_gcov_writeout in GHUnitIOS(GHTesting.o)
      ___llvm_gcov_writeout in GHUnitIOS(GHTestOperation.o)
      ...
  "_llvm_gcda_start_file", referenced from:
      ___llvm_gcov_writeout in GHUnitIOS(GHAsyncTestCase.o)
      ___llvm_gcov_writeout in GHUnitIOS(GHTest+JUnitXML.o)
      ___llvm_gcov_writeout in GHUnitIOS(GHTest.o)
      ___llvm_gcov_writeout in GHUnitIOS(GHTestGroup+JUnitXML.o)
      ___llvm_gcov_writeout in GHUnitIOS(GHTestGroup.o)
      ___llvm_gcov_writeout in GHUnitIOS(GHTesting.o)
      ___llvm_gcov_writeout in GHUnitIOS(GHTestOperation.o)
      ...
  "_llvm_gcov_init", referenced from:
      ___llvm_gcov_init in GHUnitIOS(GHAsyncTestCase.o)
      ___llvm_gcov_init in GHUnitIOS(GHTest+JUnitXML.o)
      ___llvm_gcov_init in GHUnitIOS(GHTest.o)
      ___llvm_gcov_init in GHUnitIOS(GHTestGroup+JUnitXML.o)
      ___llvm_gcov_init in GHUnitIOS(GHTestGroup.o)
      ___llvm_gcov_init in GHUnitIOS(GHTesting.o)
      ___llvm_gcov_init in GHUnitIOS(GHTestOperation.o)
      ...
ld: symbol(s) not found for architecture i386
clang: error: linker command failed with exit code 1 (use -v to see invocation)

** BUILD FAILED **


The following build commands failed:
    Ld build/Debug-iphonesimulator/Tests.app/Tests normal i386
(1 failure)

dodikk commented Feb 18, 2014

Setting the mentioned flags in the example project should fix the issue

GCC_GENERATE_TEST_COVERAGE_FILES = YES
GCC_INSTRUMENT_PROGRAM_FLOW_ARCS = YES

I think it makes sense enabling coverage flags for all configurations of the test project. Isn't it?
So I did not bother building a framework that has disabled coverage capabilities.

Sorry for not taking care about the example. Just was not sure what should be checked...

P.S. Such errors are one of the reasons I had created another target we used to discuss.

Owner

x2on commented Feb 18, 2014

And what's about projects, that are using gh-unit but don't wan't to use coverage or couldn't during some reasons?
Is it possible to set up the code that coverage is not necessary?

dodikk commented Feb 18, 2014

The coverage reports can be silently generated and then just be ignored by those gh-unit users.

In this case they should add the flags above to their test projects. I think, it should not be that hard adding these instructions to the readme or FAQ.

Alternatively, a separate backward compatible binary could be built. It is better for the users but it's some extra work for the maintainer.

Owner

x2on commented Feb 22, 2014

@gabriel what did you think?

Should we merge this incompatible change in a new 0.60 version and add the info the the README?

@x2on x2on closed this Aug 9, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment