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

Finish porting test suite to Bazel [BUILD-311] #1223

Merged
merged 2 commits into from
Sep 16, 2022
Merged

Conversation

krisukox
Copy link
Contributor

@krisukox krisukox commented Sep 14, 2022

Description

@swift-nav/devinfra

This PR adds the following bazel targets:

  • sbp-v4-test
  • sbp-cpp-v4-test
  • sbp-string-test
  • sbp-cpp-legacy-test

In the sbp-cpp-legacy-test, SbpStdioTest.ReadsSbpFiles and SbpStdioTest.WritesToSbpFiles targets have undefined behavior. Assignment of msg to payload in the MsgObsHandler::handle_sbp_msg function causes this issue.
It works better with gcc-11 but still, these two testcases shouldn't be run together. In order to run all the test cases:
CC=gcc-11 CXX=g++-11 bazel test --test_output=all //c:all --test_arg=--gtest_filter=-SbpStdioTest.WritesToSbpFiles
CC=gcc-11 CXX=g++-11 bazel test --test_output=all //c:sbp-cpp-legacy-test --test_arg=--gtest_filter=SbpStdioTest.WritesToSbpFiles

JIRA Reference

https://swift-nav.atlassian.net/browse/BUILD-311

@krisukox krisukox changed the title Finish porting test suite [BUILD-311] Finish porting test suite to Bazel [BUILD-311] Sep 15, 2022
@krisukox krisukox marked this pull request as ready for review September 15, 2022 08:53
@krisukox krisukox requested review from a team and silverjam as code owners September 15, 2022 08:53
Copy link
Contributor

@jungleraptor jungleraptor left a comment

Choose a reason for hiding this comment

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

I'm going to chalk the test failure up to the bazel submodule not being public. Not super concerning since we're just merging into a branch.

I tested locally without your command line flags and I didn't run into any errors with either gcc-11 or clang-14.

deps = [
":sbp",
"@my-googletest//:gtest_main",
],
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
],
],
tags = ["exclusive"],

This attribute will make it so that the test run serially after the other tests. I don't think it's as granular as making the individual tests themselves run in serial but it might be worth a shot.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Today, even when I run all of the tests at once, it works for me with gcc-11.
Clang-14 still doesn't work, even with the "exclusive" tag.
The behavior is so unpredictable. I will create a ticket about it.

@krisukox krisukox merged commit a25bc79 into bazel Sep 16, 2022
@krisukox krisukox deleted the krisukox/bazel-tests branch September 16, 2022 07:38
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.

2 participants