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

refactor: Rename gtest conflicting test macros #535

Merged
merged 1 commit into from
Dec 9, 2024

Conversation

hallfox
Copy link
Collaborator

@hallfox hallfox commented Dec 3, 2024

bmqtst provides several gtest-like compatibility macros, but this prevents us from using gtest or gmock directly because it uses the exact same names as the test macros from that library. Since we want to support at least using gmock in our tests, we rename them to avoid conflict.

@hallfox hallfox added the A-Testing Area: Testing label Dec 3, 2024
@hallfox hallfox requested a review from a team as a code owner December 3, 2024 18:58
@pniedzielski
Copy link
Collaborator

@hallfox DCO check failure and formatting check failure. Could you do the clang formatting fixes in another commit, so I can still easily review the first commit?

@pniedzielski pniedzielski self-requested a review December 3, 2024 20:47
@pniedzielski pniedzielski self-assigned this Dec 3, 2024
@hallfox hallfox force-pushed the rename-gtest-style-macros branch 2 times, most recently from 61068ff to feac33c Compare December 3, 2024 21:11
Copy link

@bmq-oss-ci bmq-oss-ci bot left a comment

Choose a reason for hiding this comment

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

Build 393 of commit feac33c has completed with FAILURE

@hallfox hallfox force-pushed the rename-gtest-style-macros branch 2 times, most recently from 4b24bce to 73e2eda Compare December 4, 2024 21:26
Copy link
Collaborator

@pniedzielski pniedzielski left a comment

Choose a reason for hiding this comment

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

Looks good. I see three remaining ASSERTs outside of bmqst; they don't prevent building but here they are:

src/groups/bmq/bmqp/bmqp_puteventbuilder.t.cpp
2281:    ASSERT(rawEvent.isValid());
2282:    ASSERT(rawEvent.isPutEvent());
2288:    ASSERT(putIter.isValid());

Copy link

@bmq-oss-ci bmq-oss-ci bot left a comment

Choose a reason for hiding this comment

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

Build 395 of commit 73e2eda has completed with FAILURE

@hallfox hallfox force-pushed the rename-gtest-style-macros branch 2 times, most recently from 6548a47 to e7eef53 Compare December 6, 2024 22:10
bmqtst provides several gtest-like compatibility macros, but this
prevents us from using gtest or gmock directly because it uses the exact
same names as the test macros from that library. Since we want to
support at least using gmock in our tests, we rename them to avoid conflict.

Signed-off-by: Taylor Foxhall <[email protected]>
@hallfox hallfox force-pushed the rename-gtest-style-macros branch from e7eef53 to e262c9f Compare December 6, 2024 22:20
Copy link

@bmq-oss-ci bmq-oss-ci bot left a comment

Choose a reason for hiding this comment

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

Build 403 of commit e262c9f has completed with FAILURE

Copy link
Collaborator

@pniedzielski pniedzielski left a comment

Choose a reason for hiding this comment

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

👍🏻

@hallfox hallfox merged commit 1739af0 into bloomberg:main Dec 9, 2024
35 checks passed
@hallfox hallfox deleted the rename-gtest-style-macros branch December 9, 2024 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Testing Area: Testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants