Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement tests for construct query parsing #540

Merged
merged 27 commits into from Jan 25, 2022

Conversation

RobinTF
Copy link
Collaborator

@RobinTF RobinTF commented Jan 13, 2022

WIP some design decisions need to be discussed

ASSERT_EQ(toStringView(result->first), std::string(BUFFER_SIZE, 'A'));
ASSERT_TRUE(result->second);

auto result2 = writer.get(errorCode);
ASSERT_EQ(errorCode, boost::system::error_code());
ASSERT_NE(result, boost::none);
ASSERT_TRUE(result2 != boost::none);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some unknown reason the ASSERT_NE/EQ makros cannot be used to compare boost::none with boost::optional. It seems like gtest has a problem trying to instantiate a templated function that produces a string representation of those types

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try this out, if this is a bug.

Copy link
Member

@joka921 joka921 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is absolutely the correct direction,
You can add the additional test cases for the Parser (simpler cases, but also for single functions)

src/parser/data/BlankNode.h Outdated Show resolved Hide resolved
src/parser/data/Iri.h Outdated Show resolved Hide resolved
src/parser/data/Literal.h Show resolved Hide resolved
test/SparqlAntlrParserTest.cpp Outdated Show resolved Hide resolved

EXPECT_THAT(triples[0][0], IsBlankNode("_:g_0"));
EXPECT_THAT(triples[0][1], IsVariable("?a"));
EXPECT_THAT(triples[0][2], IsBlankNode("_:g_3"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EXPECT_THAT(triples[0], {Blank(true, "0"), Var("?a"), Blank(true, "3")});
or even
expectTriple(0, {Blank(true, "0"), Var("?a"), Blank(true, "3")});

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and all Blank nodes are generated, so this can be also abstracted away

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blank(0)

test/SparqlAntlrParserTest.cpp Show resolved Hide resolved
ASSERT_EQ(toStringView(result->first), std::string(BUFFER_SIZE, 'A'));
ASSERT_TRUE(result->second);

auto result2 = writer.get(errorCode);
ASSERT_EQ(errorCode, boost::system::error_code());
ASSERT_NE(result, boost::none);
ASSERT_TRUE(result2 != boost::none);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try this out, if this is a bug.

test/SparqlAntlrParserTestHelpers.h Outdated Show resolved Hide resolved
test/SparqlAntlrParserTestHelpers.h Outdated Show resolved Hide resolved
@RobinTF RobinTF marked this pull request as ready for review January 23, 2022 18:48
Copy link
Member

@joka921 joka921 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this is really useful,
there are only some small nitpicks.

src/parser/data/BlankNode.h Outdated Show resolved Hide resolved
src/parser/data/Literal.h Outdated Show resolved Hide resolved
src/parser/data/Variable.h Outdated Show resolved Hide resolved
test/SparqlAntlrParserTest.cpp Outdated Show resolved Hide resolved
test/SparqlAntlrParserTest.cpp Outdated Show resolved Hide resolved
test/SparqlAntlrParserTest.cpp Outdated Show resolved Hide resolved
test/SparqlAntlrParserTest.cpp Outdated Show resolved Hide resolved
test/SparqlAntlrParserTestHelpers.h Outdated Show resolved Hide resolved
test/SparqlAntlrParserTestHelpers.h Outdated Show resolved Hide resolved
test/SparqlDataTypesTest.cpp Outdated Show resolved Hide resolved
Copy link
Member

@joka921 joka921 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment to wrap this up.

Copy link
Member

@joka921 joka921 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much for these extensive tests.

@joka921 joka921 merged commit facaf30 into ad-freiburg:master Jan 25, 2022
@RobinTF RobinTF deleted the construct-tests branch January 25, 2022 15:13
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.

None yet

2 participants