-
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
Add the unit test of Iterator to trace_analyzer_test #4282
Conversation
tools/trace_analyzer_test.cc
Outdated
"-analyze_merge", | ||
"-analyze_single_delete", | ||
"-analyze_range_delete", | ||
"-analyze_iterator", |
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.
For the iterator test you don't need to analyze other operations like get/put/delete/etc, right?
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.
Yeah. I will remove the other unrelated analyzing options
tools/trace_analyzer_test.cc
Outdated
Status s = env_->FileExists(trace_path); | ||
if (!s.ok()) { | ||
GenerateTrace(trace_path); | ||
} |
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.
Since the initial setup steps are the same for most of the other tests, you can pull the common code into one function, and I guess you can pass in the needed params to it.
f936c23
to
c552838
Compare
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.
zhichao-cao 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.
Thanks for the changes. lgtm.
@zhichao-cao 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.
zhichao-cao has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary: Add the unit test of Iterator (Seek and SeekForPrev) to trace_analyzer_test. The output files after analyzing the trace file are checked to make sure that analyzing results are correct. Pull Request resolved: facebook#4282 Differential Revision: D9436758 Pulled By: zhichao-cao fbshipit-source-id: 88d471c9a69e07382d9c6a45eba72773b171e7c2
Add the unit test of Iterator (Seek and SeekForPrev) to trace_analyzer_test. The output files after analyzing the trace file are checked to make sure that analyzing results are correct.
Test Plan:
make asan_check