megatest: CI should not compile/run files individually, could speedup CI by 15X

[timotheecour]

proposal

• replace when isMainModule blocks by when isMainModule or defined(testing) for all files we want to test in testament
• make testament auto-generate a "single" (more precisely: a few, see below) file test_all.nim (not checked in) that imports all files we want to test in testament, minus a blacklist
• run nim c -r test_all.nim

proof of concept

• try out the WIP PR [WIP] [do not review] single-module tester #9580 ; it takes ~3 seconds to compile and run test_all.nim versus ~50 seconds for compiling/running the files individually
  That's a ~15X speedup, but the speedup would be even larger when aggregating more files; right now it aggregates all files run by the lib category, ie:

testStdlib(r, "lib/pure/*.nim", options, cat)

(~91 files minus a blacklist of a 4 files that I commented out because it would lead to circular dependencies)

note

this is related to allow import inside block: makes N runnableExamples run N x faster, minimizes scope pollution · Issue #9300 · nim-lang/Nim in 1 aspect (compiling a single file instead of multiple files)

will that "hide" some bugs?

I doubt it would, if anything running test_all.nim should expose more bugs rather than less bugs.
Furthermore we can always add individual files if we ever discover ones that are only exhibited if they're run in isolation as opposed to via test_all.nim

A "light" version of this proposal is to first run test_all.nim, and if it passes, run the rest as we already do; the benefit being it'll fail early after 3 seconds instead of after XX seconds after the nth file fails.

what about different compiler options?

just generate several test_all_{my_hashed_option}.nim , one per unique compiler flags we're currently testing; eg:
if we currently run files f1, f2 with flags a and files f3,f4,f5 with flags b, we'll have test_all_a importing f1,f2 and test_all_b importing f3,f4,f5

what about files with discard """ blocks?

we can focus on the ones which just echo some output, and make these use doAssert which is a better practice anyways.
other files (eg that test compile errors) could be left as-is

[narimiran]

we can focus on the ones which just echo some output, and make these use doAssert which is a better practice anyways.

In my test-merging PRs I'm doing exactly that where possible, but there are (lots of) places where echo is inside of a loop so you can't use doAssert.

[timotheecour]

In my test-merging PRs I'm doing exactly that where possible

that's great, I saw that

but there are (lots of) places where echo is inside of a loop so you can't use doAssert.

can you give me a typical (not extreme) example of such a case ? it should almost always be possible (for runtime echo at least; compile time is only sometimes possible)

eg:

proc foo()=
  for i in 0..<3:
    echo bar(i)

=>

proc foo()=
  var msg = ""
  for i in 0..<3:
    msg.add $bar(i)
  doAssert msg == ...

or even putting var msg = " at module top-level (least preferred, but maybe easiest transition from echo-style tests)

[Araq]

tcomputedgoto comes to mind.

[timotheecour]

tcomputedgoto comes to mind.

see #9597 ; I'm using tcomputedgoto.nim as 1st example of test transformed to doAssert in that PR

[Araq]

So in other words, you automate what @narimiran did but tests can stay small for individual testing during development. I like it.

[timotheecour]

/cc @Araq @narimiran
@krux02 closed #9597 with tester can be adjusted to collect the output of several tests. You don't need a fake echo for that

how about the following then, which works regardless a test has a discard section or just uses doAssert (excluding tests that are intended to fail with compiler error, see footnote)

• test runner collects (automatically, so it's guaranteed to stay in sync when tests are added/removed) all regular [1] tests (possibly minus a blacklist) into a /tmp/alltests_autogen.nim as follows:

echo "testrunner_before test1.nim"
import "patho/test1"
echo "testrunner_after test1.nim"

echo "testrunner_before test2.nim"
import "patho/test2"
echo "testrunner_after test2.nim"

...

• then it collects all output from stdout and assigns these to a Table[testName, testOutput] thanks to the testrunner_before/testrunner_after markers
• then we report which tests succeed/fail based on expected output from discard section
• we may not even need test categories anymore, as running all tests at once as above will likely be faster than splitting into categories; however we can still use the categories, if needed, for reporting test statistics per category

footnotes

[1] regular test meaning: the test is expected to succeed, ie it doesn't expect it to fail with a specific compiler error (and we can reuse logic in testament by parsing the discard test section for that)

note

if for some reason we don't want to stop on 1st error, we can convert tests from this form:

discard """
...
"""
echo "foobar"
...

to this:

discard """
...
"""
proc main=
  echo "foobar"

and adjust /tmp/alltests_autogen.nim as follows:

import "patho/test1"
import "patho/test2"
...
var numFailures = 0
for test in (test1, test2,...):
  try:
    echo "testrunner_before " & $test
    test.main()
  except:
    numFailures.inc # add other failure info for test stats generation if needed
  finally:
    echo "testrunner_after " & $test
# then report success/failures based on numFailures and above logic based on `Table[testName, testOutput]` 

EDIT

hmm, I just tried, one issue is that echo in /tmp/alltests_autogen.nim are printed AFTER all imported modules's top-level code, so we need something a bit different; the following seems to work:

EDIT

ok I think i found a solution, stay tuned

[timotheecour]

update:
here's what I tried:

a new module testament/runall.nim collects all tests and generates 1 wrapper per test (eg /tmp/D20181117T141313/trunall_0_tests_stdlib_tstrset.nim) and 1 module that aggregates all wrappers:

/tmp/D20181117T141313/z01.nim:

# autogenerated; v1
import "/tmp/D20181117T141313/trunall_0_tests_stdlib_tstrset.nim"
import "/tmp/D20181117T141313/trunall_1_tests_stdlib_tunidecode.nim"
import "/tmp/D20181117T141313/trunall_2_tests_stdlib_tcputime.nim"
import "/tmp/D20181117T141313/trunall_3_tests_stdlib_thttpcore.nim"
...

/tmp/D20181117T141313/trunall_0_tests_stdlib_tstrset.nim:
```nim
static: echo "[0/72] ct_before /Users/timothee/git_clone/nim/Nim/tests/stdlib/tstrset.nim"
echo "[0/72] rt_before /Users/timothee/git_clone/nim/Nim/tests/stdlib/tstrset.nim"
include "/Users/timothee/git_clone/nim/Nim/tests/stdlib/tstrset.nim"
static: echo "ct_after /Users/timothee/git_clone/nim/Nim/tests/stdlib/tstrset.nim"
echo "rt_after /Users/timothee/git_clone/nim/Nim/tests/stdlib/tstrset.nim"

I ran into other issues while doing that, eg:

• testament ignores tests that fail at runtime (for tests without discard section) · Issue #9749 · nim-lang/Nim
• for a few modules, running import A; import B crashes even though both work individually (memory corrupted; possibly by try/catch using import segfaults #9752, memory corruption in tmarshall.nim #9754)

but overall this works.

for the tests in tests/stdlib (minus a blacklist of 4 files, to avoid above issue), compiling/running tests individually took 75 seconds, vs 13 seconds for compiling/running the aggregate; and stdout delimits when a module starts/ends thanks to the wrapper trick I used, from that we can compare against discard output block

[Araq]

This has been implemented and merged. Speedups are not that great but for different reasons we can address later.

[timotheecour]

/cc @Araq
actually I just measured timing, not very scientific but, but also not as good as what I measured earlier:

# with megatest logic (ie current nim)
nim c -r testament/tester all # 390 seconds

# patched code to only run isJoinableSpec tests, and using megatest
nim c -r testament/tester all # 34 seconds

# patched code to only run isJoinableSpec tests, and running them individually
nim c -r testament/tester all # 74 seconds

Speedups are not that great but for different reasons we can address later.

any thoughts?

[Araq]

Travis spends most of its time in ./koch csource afaict. But the GC issue you found seems also to affect this greatly. :-)

[timotheecour]

btw just edited numbers above, it's not TOO bad: 34 seconds vs 74 seconds using a more focused comparison