Repository navigation
src: add c++ test coverage #1193
Description
Meta-issue for something that was discussed in today's TC meeting: test coverage for C++ code that is only tested indirectly now.
Strawman proposal: bundle gtest, make it part of make test and start churning out src/*_unittest.cc tests like chromium has.
Problem: most of the code in src/ is not very modular right now. Turning it into something that is easily testable will no doubt create a lot of churn. Probably unavoidable but it's acceptable to me.
/cc @indutny @piscisaureus @trevnorris - if we can get some consensus on this, I volunteer to do the initial work.
Activity
indutny commentedon Mar 18, 2015
It is very interesting thing. What I am mostly curious about is - how much of the stuff will we be able to test within the C++? It seems to me that many if not most of the code is tightly bound to the JS.
The second question is even more interesting :) Do we want it to be cross-platform? Or can we do some raw
socket()machinery in it? Like working with fds, and stuff like that.indutny commentedon Mar 18, 2015
+1 for this in general, though
bnoordhuis commentedon Mar 18, 2015
That's why I predict there will be a fair bit of churn, to break out the meat from the glue code. I think it's alright when tests set up a VM where it makes sense.
It should be cross-platform, with the platform-dependent bits guarded by #ifdef statements. Just like libuv, really. :-)
indutny commentedon Mar 18, 2015
Sounds awesome! Go ahead with it :)
jbergstroem commentedon Mar 18, 2015
Good idea indeed. Is there perhaps a
.gypfor gtest somewhere? I'd really despise having to pull cmake or auto* into the dependency chain. Another plan could be making running these tests optional and assume you can handle installing gtest yourself?bnoordhuis commentedon Mar 19, 2015
I don't think there is one currently but it should be pretty easy to get it gypified. gtest core is only a handful of files.
deps: check in gtest, add util unit test
trevnorris commentedon Mar 19, 2015
I'm fine with this. Will it mean
make test-addonwill be removed/unneeded?bnoordhuis commentedon Mar 19, 2015
Not right away but it could, longer term. Not the part of
make test-addonthat scrapes the API documentation, though.Fishrock123 commentedon Apr 11, 2015
Can this be closed now? :)
Edit: Assuming it's open until we add more coverage?
bnoordhuis commentedon Apr 11, 2015
Yes, it's fixed (if you can call it that) in 0080788. Adding tests for code in src/ is an ongoing process.