Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

feat(bindings/C): support BDD tests using GTest #2254

Merged
merged 15 commits into from
May 11, 2023
Merged

feat(bindings/C): support BDD tests using GTest #2254

merged 15 commits into from
May 11, 2023

Conversation

xyjixyjixyji
Copy link
Contributor

This PR does

  • add gtest dependency
  • implement BDD Tests with gtest
  • remove basicio.c

@xyjixyjixyji
basic framework for BDD tests in Gtest
d3fd924 
Signed-off-by: Ji-Xinyou <jerryji0414@outlook.com>
@xyjixyjixyji
remove basicio.c, replace with bdd test equivalent GTest
2e5e0f8 
Signed-off-by: Ji-Xinyou <jerryji0414@outlook.com>
@xyjixyjixyji
some comments
89779ce 
Signed-off-by: Ji-Xinyou <jerryji0414@outlook.com>
@xyjixyjixyji
makefile minor change
9bc0f09 
Signed-off-by: Ji-Xinyou <jerryji0414@outlook.com>
@xyjixyjixyji
Copy link
Contributor Author

Related #2185

@xyjixyjixyji
Copy link
Contributor Author

xyjixyjixyji commented May 11, 2023 •
edited
Loading

The typo check failed on GTest headers.

Followup:
I remove the GTest headers for this PR, which requires developers to have gtest headers on their own machines. Since we have to link with libgtest anyway, I think it would be fine.

I will explain this in detail on the contributing doc PR.

@xyjixyjixyji
header
09e1ab5 
Signed-off-by: Ji-Xinyou <jerryji0414@outlook.com>
@xyjixyjixyji
remove gtest headers
ce7f34d 
Signed-off-by: Ji-Xinyou <jerryji0414@outlook.com>
@xyjixyjixyji
minor
80b8ed5 
Signed-off-by: Ji-Xinyou <jerryji0414@outlook.com>
@xyjixyjixyji

This comment was marked as resolved.

# to view
@xyjixyjixyji
minor
df1b926 
Signed-off-by: Ji-Xinyou <jerryji0414@outlook.com>
Xuanwo
Xuanwo reviewed May 11, 2023
View reviewed changes
@xyjixyjixyji
header
92994ae 
Signed-off-by: Ji-Xinyou <jerryji0414@outlook.com>
@Xuanwo
Copy link
Member

Xuanwo commented May 11, 2023

We need a workflow to make sure it works.

@xyjixyjixyji
gha
b509839 
Signed-off-by: Ji-Xinyou <jerryji0414@outlook.com>
@xyjixyjixyji

This comment was marked as resolved.

# to view
@xyjixyjixyji
gha
034376b 
Signed-off-by: Ji-Xinyou <jerryji0414@outlook.com>
@xyjixyjixyji
manually setup c/cpp
0c1f8fa 
Signed-off-by: Ji-Xinyou <jerryji0414@outlook.com>
@xyjixyjixyji

This comment was marked as resolved.

# to view
@xyjixyjixyji
run test and fix build
b2f40a0 
Signed-off-by: Ji-Xinyou <jerryji0414@outlook.com>
@xyjixyjixyji
Copy link
Contributor Author

Works perfectly now

Xuanwo
Xuanwo reviewed May 11, 2023
View reviewed changes
@xyjixyjixyji
remove cpp setup, run test on make test
b68754c 
Signed-off-by: Ji-Xinyou <jerryji0414@outlook.com>
@xyjixyjixyji
minor
b9ad0f6 
Signed-off-by: Ji-Xinyou <jerryji0414@outlook.com>
Xuanwo
Xuanwo approved these changes May 11, 2023
View reviewed changes
Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

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

Great, thanks!

Xuanwo
Xuanwo approved these changes May 11, 2023
View reviewed changes
Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

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

Great, thanks!

@Xuanwo Xuanwo merged commit 5d89060 into apache:main May 11, 2023
@Xuanwo Xuanwo mentioned this pull request May 23, 2023
Merged
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@Xuanwo Xuanwo Xuanwo approved these changes

@PsiACE PsiACE Awaiting requested review from PsiACE

@ClSlaid ClSlaid Awaiting requested review from ClSlaid

@sundy-li sundy-li Awaiting requested review from sundy-li

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@xyjixyjixyji @Xuanwo