Skip to content

clar: fix parsing of test suite prefixes#3881

Merged
carlosmn merged 1 commit into
libgit2:masterfrom
pks-t:pks/fix-clar-suite-prefix-computation
Aug 5, 2016
Merged

clar: fix parsing of test suite prefixes#3881
carlosmn merged 1 commit into
libgit2:masterfrom
pks-t:pks/fix-clar-suite-prefix-computation

Conversation

@pks-t
Copy link
Copy Markdown
Member

@pks-t pks-t commented Aug 4, 2016

When passing in a specific suite which should be executed by clar
via -stest::suite, we try to parse this string and then include
all tests contained in this suite. This also includes all tests
in sub-suites, e.g. 'test::suite::foo'.

In the case where multiple suites start with the same string,
for example 'test::foo' and 'test::foobar', we fail to
distinguish this correctly. When passing in -stest::foobar,
we wrongly determine that 'test::foo' is a prefix and try to
execute all of its matching functions. But as no function
will now match 'test::foobar', we simply execute nothing.

To fix this, we instead have to check if the prefix is an actual
suite prefix as opposed to a simple string prefix. We do so by by
inspecting if the first two characters trailing the prefix are
our suite delimiters '::', and only consider the filter as
matching in this case.

When passing in a specific suite which should be executed by clar
via `-stest::suite`, we try to parse this string and then include
all tests contained in this suite. This also includes all tests
in sub-suites, e.g. 'test::suite::foo'.

In the case where multiple suites start with the same _string_,
for example 'test::foo' and 'test::foobar', we fail to
distinguish this correctly. When passing in `-stest::foobar`,
we wrongly determine that 'test::foo' is a prefix and try to
execute all of its matching functions. But as no function
will now match 'test::foobar', we simply execute nothing.

To fix this, we instead have to check if the prefix is an actual
suite prefix as opposed to a simple string prefix. We do so by by
inspecting if the first two characters trailing the prefix are
our suite delimiters '::', and only consider the filter as
matching in this case.
@pks-t
Copy link
Copy Markdown
Member Author

pks-t commented Aug 4, 2016

Btw, forgot to mention in the commit message: this actually hits us when trying to execute ./libgit2_clar -sonline::fetchhead, which will then try to execute the 'online::fetch' suite.

@ethomson
Copy link
Copy Markdown
Member

ethomson commented Aug 4, 2016

Interesting! Would you mind opening this also against https://github.com/vmg/clar so that we can make sure that it lands upstream?

@pks-t
Copy link
Copy Markdown
Member Author

pks-t commented Aug 4, 2016

Sure. Didn't know upstream actually existed.

@pks-t
Copy link
Copy Markdown
Member Author

pks-t commented Aug 4, 2016

See clar-test/clar#76

@pks-t
Copy link
Copy Markdown
Member Author

pks-t commented Aug 5, 2016

This has been merged upstream by @vmg

@carlosmn carlosmn merged commit c550c92 into libgit2:master Aug 5, 2016
@pks-t pks-t deleted the pks/fix-clar-suite-prefix-computation branch August 8, 2016 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants