Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

Fix test coverage merging with the test_runner #1629

Merged
merged 1 commit into from Aug 12, 2016

Conversation

wilzbach
Copy link
Member

@wilzbach wilzbach commented Aug 12, 2016

There is a weird case with std.stdio where doTest is called 4x and for the first three times currently without the merge flag been set.
This is a simple fix that just moves the setting of the merge flag in the static constructor, however if someone wants to look deeper into this, here's a strip down version of the test_runner.

import core.runtime, core.time : MonoTime;
import core.stdc.stdio;

ModuleInfo* getModuleInfo(string name)
{
    foreach (m; ModuleInfo)
        if (m.name == name) return m;
    assert(0, "module '"~name~"' not found");
}

bool testModules()
{
    printf("testModules\n");
    doTest(getModuleInfo(Runtime.args[1]));
    return true;
}

void doTest(ModuleInfo* moduleInfo)
{
    if (auto fp = moduleInfo.unitTest)
    {
        printf("fp");
        fp();
    }
}

shared static this()
{
    Runtime.moduleUnitTester = &testModules;
}

void main()
{
    version(D_Coverage)
    {
        import core.runtime : dmd_coverSetMerge;
        dmd_coverSetMerge(true);
    }
}

For std.json or any other module it will print as expected

> generated/linux/debug/64/unittest/test_runner std.json
testModules
fp
merge: 1

where merge is an additional log output in the static deconstructor of cover.d which just logs the current merge flag. Now with std.stdio for some reason doTest is called 4x

> generated/linux/debug/64/unittest/test_runner std.stdio
testModules
fp
merge: 0
fp
merge: 0
fp
merge: 0
fp
merge: 1

For reference:

dlang/phobos#4719

@MartinNowak
Copy link
Member

Auto-merge toggled on

@wilzbach
Copy link
Member Author

Maybe the test runner is linked against the same shared library?

Hmm the Makefile seems to do everything correctly:

../dmd/src/dmd -conf= -I../druntime/import  -w -dip25 -m64  -g -debug -cov -ofgenerated/linux/debug/64/unittest/test_runner ../druntime/src/test_runner.d -Lgenerated/linux/debug/64/unittest/libphobos2-ut.so -defaultlib= -debuglib=

where ibphobos2-ut.so is:

../dmd/src/dmd -conf= -I../druntime/import  -w -dip25 -m64 -fPIC -g -debug -cov -shared -unittest -ofgenerated/linux/debug/64/unittest/libphobos2-ut.so generated/linux/debug/64/unittest/std/array.o
...

@MartinNowak
Copy link
Member

Seems to happen because of the runForked in std.stdio, not sure how forking would disable the merge option.

version(D_Coverage)
{
import core.runtime : dmd_coverSetMerge;
dmd_coverSetMerge(true);
Copy link
Member

Choose a reason for hiding this comment

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

Well, this has always been wrong anyhow, b/c unittests are run from the runtime initialization before entering main.

@wilzbach
Copy link
Member Author

Seems to happen because of the runForked in std.stdio, not sure how forking would disable the merge option.

That does make sense partially sense, because then the other processes are closed before the main method is run and thus are without merge flag :/

@MartinNowak MartinNowak merged commit 184b68e into dlang:master Aug 12, 2016
@MartinNowak
Copy link
Member

That does make sense partially sense, because then the other processes are closed before the main method is run and thus are without merge flag :/

Ah of course, you set the merge flag after running the tests/forking. Well then this is the correct fix here.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants