-
Notifications
You must be signed in to change notification settings - Fork 169
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
[Refactoring] Apply test fixture for each test case #3716
[Refactoring] Apply test fixture for each test case #3716
Conversation
📝 TAOS-CI Version: 1.5.20200925. Thank you for submitting PR #3716. Please a submit 1commit/1PR (one commit per one PR) policy to get comments quickly from reviewers. Your PR must pass all verificiation processes of cibot before starting a review process from reviewers. If you are new member to join this project, please read manuals in documentation folder and wiki page. In order to monitor a progress status of your PR in more detail, visit http://nnstreamer.mooo.com/. |
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.
@again4you, 💯 All CI checkers are successfully verified. Thanks.
|
||
model_file = g_build_filename (root_path, "tests", "test_models", "models", | ||
"mobilenet_v1_1.0_224_quant.tflite", NULL); | ||
if (!g_file_test (model_file, G_FILE_TEST_EXISTS)) { |
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.
When model file is not in model directory or reading file content fails (line 62 and 68), all testcase should be stopped.
How about using ASSERT_TRUE() for this case?
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.
Good advice! I updated the code as below.
ASSERT_TRUE (g_file_test (model_file, G_FILE_TEST_EXISTS));
...
ASSERT_TRUE (g_file_get_contents (data_file, (gchar **) & input.data, &length, NULL));
ASSERT_TRUE (length == input.size);
To remove duplicate code in each test case, this patch applies the test fixture framework. Moreover, g_autofree macro is used for allocated space to be freed automatically no matter test case is passed or not. Signed-off-by: Sangjung Woo <sangjung.woo@samsung.com>
To remove duplicate code in each test case, this patch applies the test fixture framework of Google Test. It increases maintainability and modifiability. Signed-off-by: Sangjung Woo <sangjung.woo@samsung.com>
d38323b
to
3de6f7c
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.
@again4you, 💯 All CI checkers are successfully verified. Thanks.
Submit historyv2
|
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.
LGTM!
Summary
To increase maintainability and modifiability, this patch applies the test fixture framework of Google Test.
It removes duplicate code in each test case and makes a
SetUp()
andTearDown()
method for each test setup and teardown.Submit history
v2
v1
Before
Data structure and variables in
unittest_rate.cc
are defined asGlobal
scope and each test case has duplicate code as below.After
This patch applies the test fixture framework as below. So duplicate code is moved to
SetUp()
andTearDown()
methods and is used for each test case. It causes increasing maintainability and modifiability.