-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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
[FB Internal] Directly use unit test tempalte buck #6926
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@siying has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
@siying has updated the pull request. Re-import the pull request |
1 similar comment
@siying has updated the pull request. Re-import the pull request |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@siying has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
@siying has updated the pull request. Re-import the pull request |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@siying has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
@siying has updated the pull request. Re-import the pull request |
@siying has updated the pull request. Re-import the pull request |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@siying has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@siying has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this would have made more sense explicitly reverting #6858, but this is OK.
Spelling is only thing I see to fix.
buckifier/buckify_rocksdb.py
Outdated
@@ -171,7 +159,10 @@ def generate_targets(repo_path, deps_map): | |||
src_mk.get("TEST_LIB_SOURCES", []) + | |||
src_mk.get("EXP_LIB_SOURCES", []) + | |||
src_mk.get("ANALYZER_LIB_SOURCES", []), | |||
[":rocksdb_lib", ":rocksdb_third_party_gtest"]) | |||
[":rocksdb_lib"], | |||
extra_exteranl_deps=""" + [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extra_exteranl_deps misspelled (consistently) in four places
Summary: Make RocksDB run a predefined unit test so that it can be integrated with better tools. Test Plan: Watch tests Fix a bug
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@siying has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
@siying has updated the pull request. Re-import the pull request |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@siying has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine except for internal tests
This pull request has been merged in 2e7070b. |
This reverts commit 2e7070b. It is necessary because `cpp_unittest` is not compatible with "db/c_test.c". Test Plan: built/ran test with buck.
Summary: Make RocksDB run a predefined unit test so that it can be integrated with better tools.
Test Plan: Watch tests