Add coverage tooling, update test tooling and improve coverage #24
Conversation
DifferentialOrange
force-pushed
the
DifferentialOrange/no1seman-coverage
branch
2 times, most recently
from
fedcbf7 to
7a74b4c
Compare
DifferentialOrange
force-pushed
the
DifferentialOrange/no1seman-coverage
branch
from
7a74b4c to
5fd4155
Compare
|
|
||
| .PHONY: lint | ||
| lint: | ||
| if [ ! -d ".rocks" ]; then make .rocks; fi |
There was a problem hiding this comment.
That's a bit strange. You can make .rocks non-phony target to achieve the same behaviour. But, it seems, you don't want, because it may miss some dependencies (say, after manual tarantoolctl rocks make). That's minor, though.
There was a problem hiding this comment.
I borrowed it from original PR, but I think the motivation was the same as your assumption.
There was a problem hiding this comment.
I would do it so:
.rocks/bin/luacheck
tarantoolctl rocks install luacheck <ver>
.rocks/bin/luatest
tarantoolctl rocks install luatest <ver>
.rocks/bin/luacov
tarantoolctl rocks install luacov <ver>
.PHONY: dependencies
dependencies:
tarantoolctl rocks make
.PNOHY: dependencies-test
depdendencies-test: depdendencies .rocks/bin/luacheck .rocks/bin/luatest .rocks/bin/luacov
true
But simpler is better and if everything typically works okay, I propose to don't make things more complex.
Makefile
Outdated
| .PHONY: build | ||
| build: | ||
| if [ ! -d ".rocks" ]; then make .rocks; fi | ||
| tarantoolctl rocks make | ||
| tarantoolctl rocks pack graphql scm-1 |
There was a problem hiding this comment.
Why we may need to create .all.rock for scm-1?
There was a problem hiding this comment.
I borrowed it from original PR, but I also were in several situations when I have found a bug in a rock package with my application and after drafting a fix I wanted to ensure that issue with my application is resolved by installing scm-1 and running some tests (for me it was tarantool/metrics).
There was a problem hiding this comment.
Okay, but why do you need to pack it for that? Do you use a rock tarball for deployment within an application?
There was a problem hiding this comment.
Well, to install myrock to app/.rocks, I move packed rock to app and use tarantoolctl rocks install ./myrock-scm-1.all.rock. I don't know how to transfer it another way.
Last entries line in multiline table considered as non-covered by luacov if it has no trailing comma. See more about the issue in luacov repo: lunarmodules/luacov#52 Based on PR #18 by @no1seman
DifferentialOrange
force-pushed
the
DifferentialOrange/no1seman-coverage
branch
from
5fd4155 to
ba8857c
Compare
Rework of #18 . No major changes were introduced: Tarantool bump in tests, using Makefile in tests and separation of lint and test stage in Makefile, minor syntax reworks. PR was separated to 3 commits with descriptions (where they are needed).
Commentary
"Ugly" trailing commas for one-liner tables in the first version of PR seems to be irrelevant to lunarmodules/luacov#52 issue: they considered covered even without extra commas:
