Switch to CircleCi

[wilzbach]

Follow-up to dlang/druntime#1625 and dlang/dmd#6022.

Short summary

• Travis clogs our PR queue -> use CircleCi for coverage testing
• The coverage testing script now allows to run for 64-bit & 32-bit
• moved style checks to the posix.mak as style target (hopefully more friendly to developers) - it bootstraps dscanner too (CC @Hackerpilot)
• this now uses the test_runner to generate the coverage data, this means that (a) coverage across modules will be detected, but (b) that we don't have a CI anymore that check whether all individual modules build (e.g. Fix cycles in Phobos #4493 or issue 16291) CC @schveiguy.

CC @MartinNowak

[MartinNowak]

Single module tests are there for development, not that important to run them.

[andralex]

@MartinNowak leaving this to you

[wilzbach]

@MartinNowak leaving this to you

@andralex we already agreed on this move, but @MartinNowak lacks the permission to enable CircleCi.
See also: dlang/druntime#1625

[codecov-io]

Current coverage is 88.75% (diff: 100%)

Merging #4719 into master will increase coverage by <.01%

@@             master      #4719   diff @@
==========================================
  Files           121        121          
  Lines         74070      74070          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          65737      65740     +3   
+ Misses         8333       8330     -3   
  Partials          0          0          

Powered by Codecov. Last update 41df329...ba5f55a

[wilzbach]

Merging #4719 into master will decrease coverage by 43.39%

So I inserted some log output in rt/cover.d to see what's going on here (it's not a concurrency issue) and only printed some values (global merge flag and maximal counts seen in a line) if the filename was std.json and then executed the test_runner steps manually.

So for example running std.json works as expected:

> generated/linux/debug/64/unittest/test_runner std.json
0.001s PASS debug64 std.json
merge: 1
counts: 627

and indeed if I run it again the coverage gets merge and thus doubled:

> generated/linux/debug/64/unittest/test_runner std.json
0.001s PASS debug64 std.json
merge: 1
counts: 1254

and if i run another module it is pertained:

generated/linux/debug/64/unittest/test_runner std.container.slist
0.003s PASS debug64 std.container.slist
merge: 1
counts: 1254

However once std.stdio (this is the only module with this weird behavior) is run I get this weird output - there are three more calls with std.json happening and for those the global merge flag is off.

generated/linux/debug/64/unittest/test_runner std.stdio
merge: 0
counts: 0
merge: 0
counts: 0
merge: 0
counts: 0
0.832s PASS debug64 std.stdio
merge: 1
counts: 0

Has anyone an idea what's happening here?

[MartinNowak]

Has anyone an idea what's happening here?

Maybe a bug in the merge code? Could be sth. like those other modules are initialized with an empty coverage array which then deletes the existing information.

[MartinNowak]

[do not merge]

BTW, gitlab has the nice [WIP] or WIP convention where they prevent an PR with that prefix from being merged over the web interface.

[MartinNowak]

tabs

[MartinNowak]

Issue 16379 – merging coverage sometimes overwrites existing results

[wilzbach]

Maybe a bug in the merge code? Could be sth. like those other modules are initialized with an empty coverage array which then deletes the existing information.

I think I found the problem. In test runner doTest is executed 4x for std.stdio, hence I submitted a simple fix (s.t. we move on for now) and provided more detailed explanation on how to reproduce.

[MartinNowak]

Looks good now?

[wilzbach]

I think I found the problem. In test runner doTest is executed 4x for std.stdio

Thanks @MartinNowak for the quick-deployment of the fix :)

Merging #4719 into master will decrease coverage by 1.07%
...

• Hits 65738 67711 +1973
• Misses 8332 9515 +1183

That's weird (I expected to get more hits because with the test_runner we should see coverage of code executed across modules, but we shouldn't get 1.2K new, missed lines).
-> I will have a deeper look at the diff at CodeCov :/

[wilzbach]

-> I will have a deeper look at the diff at CodeCov :/

Hmm there still seem to be some issues.
For example for the last part of std.digest.digest we get different outputs

1) Current output with test_runner,

e.g. generated/linux/debug/64/unittest/test_runner std.digest.digest

       |        else static if (hasPeek!T)
       |        {
       |            @trusted ubyte[] peek(ubyte[] buf) const
       |            in
0000000|            {
0000000|                assert(buf.length >= this.length);
       |            }
       |            body
       |            {
0000000|                enum string msg = "Buffer needs to be at least " ~ digestLength!(T).stringof ~ " bytes " ~
       |                    "big, check " ~ typeof(this).stringof ~ ".length!";
0000000|                asArray!(digestLength!T)(buf, msg) = _digest.peek();
0000000|                return buf[0 .. digestLength!T];
       |            }
       |
       |            @trusted ubyte[] peek() const
       |            {
0000000|                enum len = digestLength!T;
0000000|                auto buf = new ubyte[len];
0000000|                asArray!(digestLength!T)(buf) = _digest.peek();
0000000|                return buf;
       |            }
       |        }
       |}
       |
       |///
       |unittest
       |{
       |    import std.digest.md;
       |    //Simple example
      1|    auto hash = new WrapperDigest!MD5();
      1|    hash.put(cast(ubyte)0);
      1|    auto result = hash.finish();
       |}
       |
       |///
       |unittest
       |{
       |    //using a supplied buffer
       |    import std.digest.md;
      1|    ubyte[16] buf;
      1|    auto hash = new WrapperDigest!MD5();
      1|    hash.put(cast(ubyte)0);
      1|    auto result = hash.finish(buf[]);
       |    //The result is now in result (and in buf). If you pass a buffer which is bigger than
       |    //necessary, result will have the correct length, but buf will still have it's original
       |    //length
       |}
       |
       |@safe unittest
       |{
       |    // Test peek & length
       |    import std.digest.crc;
      1|    auto hash = new WrapperDigest!CRC32();
      1|    assert(hash.length == 4);
      1|    hash.put(cast(const(ubyte[]))"The quick brown fox jumps over the lazy dog");
      1|    assert(hash.peek().toHexString() == "39A34F41");
      1|    ubyte[5] buf;
      1|    assert(hash.peek(buf).toHexString() == "39A34F41");
       |}

2) Previous output or make -f posix.mak std/digest/digest.test

       |        else static if (hasPeek!T)
       |        {
       |            @trusted ubyte[] peek(ubyte[] buf) const
       |            in
       |            {
       |                assert(buf.length >= this.length);
       |            }
       |            body
       |            {
      1|                enum string msg = "Buffer needs to be at least " ~ digestLength!(T).stringof ~ " bytes " ~
       |                    "big, check " ~ typeof(this).stringof ~ ".length!";
      1|                asArray!(digestLength!T)(buf, msg) = _digest.peek();
      1|                return buf[0 .. digestLength!T];
       |            }
       |
       |            @trusted ubyte[] peek() const
       |            {
      1|                enum len = digestLength!T;
      1|                auto buf = new ubyte[len];
      1|                asArray!(digestLength!T)(buf) = _digest.peek();
      1|                return buf;
       |            }
       |        }
       |}
       |
       |///
       |unittest
       |{
       |    import std.digest.md;
       |    //Simple example
      1|    auto hash = new WrapperDigest!MD5();
      1|    hash.put(cast(ubyte)0);
      1|    auto result = hash.finish();
       |}
       |
       |///
       |unittest
       |{
       |    //using a supplied buffer
       |    import std.digest.md;
      1|    ubyte[16] buf;
      1|    auto hash = new WrapperDigest!MD5();
      1|    hash.put(cast(ubyte)0);
      1|    auto result = hash.finish(buf[]);
       |    //The result is now in result (and in buf). If you pass a buffer which is bigger than
       |    //necessary, result will have the correct length, but buf will still have it's original
       |    //length
       |}
       |
       |@safe unittest
       |{
       |    // Test peek & length
       |    import std.digest.crc;
      1|    auto hash = new WrapperDigest!CRC32();
      1|    assert(hash.length == 4);
      1|    hash.put(cast(const(ubyte[]))"The quick brown fox jumps over the lazy dog");
      1|    assert(hash.peek().toHexString() == "39A34F41");
      1|    ubyte[5] buf;
      1|    assert(hash.peek(buf).toHexString() == "39A34F41");
       |}

[wilzbach]

btw something that we should also fix is that even though the absolute coverage would decrease, it triggers no warning.

[wilzbach]

Single module tests are there for development, not that important to run them.

Judging by this recent discussion it's probably nice to run them too, s.t. such random discoveries don't happen again.

[andralex]

@MartinNowak what steps do we need to take to grant you appropriate rights?

[wilzbach]

@MartinNowak what steps do we need to take to grant you appropriate rights?

Thanks to the help of @WalterBright and @CyberShadow we have already enabled CircleCi (see the checks box here), but this PR is pending because we have some weird issues with coverage reporting from test_runner, but I think i will change this back to just run individual module tests for now.

[wilzbach]

this PR is pending because we have some weird issues with coverage reporting from test_runner, but I think i will change this back to just run individual module tests for now.

Rebased to run in the "old mode" (individual module test), s.t. other PRs aren't displayed as broken and we have more time & peace to dig into this issue.

(Looking at this NG discussion we might want to keep the individual module testing anyways).

[dnadlinger]

Needs the same fix as dmd#6066. :)

[wilzbach]

Needs the same fix as dmd#6066. :)

Done. Thx. Are you already submitting one for druntime?

[dnadlinger]

Auto-merge toggled on

[dnadlinger]

Merging this, as CircleCI is currently just red.

[dnadlinger]

Are you already submitting one for druntime?

Nope. Please do.