Add Google Test support #203

ngie-eign · 2019-04-03T02:05:19Z

This change adds fine grained execution of Google Test test programs.

First, the Google Test test is executed with --gtest_list_tests. Next,
based on the output from --gtest_list_tests, the testcases are run
individually. The output from each unique testcase is based on the
standard output it provides, per the test output protocol defined in
the GoogleTest docs on github [1], [2], and instrumented via various
demo programs I created, which can be found on GitHub
here.

This support is a very rough cut to provide an initial working
integration effort. There're additional improvements that can be made by
leveraging either the JSON or XML structured output format, instead of
scraping standard output using beginning and ending sentinels to search for
regular expressions. In order to do that though without reinventing the wheel,
Kyua would need to rely on an external JSON or XML library.

This test interface supports fine grained execution of test programs
like the ATF test interface, but matches behavior of plain/TAP interfaces by
instead supporting metadata passing via $TEST_ENV_ prefixed environment
variables.

This support adds additional tests for verifying pass, fail, skip (will
be available in version 1.9.0 and is available in FreeBSD base's version
of Google Test), and disabled result determination, using mock output and a mock
test program (engine/googletest_helpers), for parity with other test formats
(ATF, plain, TAP). The helper test program purposely avoids relying on the
GoogleTest test infrastructure, in order to limit Kyua's dependencies.

Signed-off-by: Enji Cooper yaneurabeya@gmail.com

ngie-eign · 2019-04-03T02:11:38Z

CC: @asomers, @emaste

asomers · 2019-04-03T03:00:21Z

🎉
One problem, though. When I use the new googletest test program, "kyua report" doesn't work.

> /home/somers/src/github/jmmv/kyua/kyua report
kyua: E: Column 'result_reason' is not a string (sqlite db: /home/somers/.kyua/store/results.usr_tests_sys_fs_fusefs.20190403-025838-736040.db).

ngie-eign · 2019-04-03T03:09:19Z

🎉
One problem, though. When I use the new googletest test program, "kyua report" doesn't work.
> /home/somers/src/github/jmmv/kyua/kyua report
kyua: E: Column 'result_reason' is not a string (sqlite db: /home/somers/.kyua/store/results.usr_tests_sys_fs_fusefs.20190403-025838-736040.db).

Hmmm... that's strange. I'll see if I can reproduce that.

asomers · 2019-04-03T14:18:59Z

"kyua report" still give me the same error even with your newest patch.

ngie-eign · 2019-04-03T16:39:59Z

"kyua report" still give me the same error even with your newest patch.

I suspect it's one of the cases involving the skipped or disabled status. Need to do some more digging here with your branch..

ngie-eign · 2019-04-05T18:00:11Z

As I suspected, the issue is that GTEST_SKIP is being called without a reason:

$ sudo kyua test -k /usr/tests/sys/fs/fusefs/Kyuafile write:Write.append
write:Write.append  ->  skipped  [0.017s]

Results file id is usr_tests_sys_fs_fusefs.20190405-171435-875837
Results saved to /root/.kyua/store/results.usr_tests_sys_fs_fusefs.20190405-171435-875837.db

1/1 passed (0 failed)
$ sudo kyua report --results-file=/root/.kyua/store/results.usr_tests_sys_fs_fusefs.20190405-171435-875837.db
kyua: E: Column 'result_reason' is not a string (sqlite db: /root/.kyua/store/results.usr_tests_sys_fs_fusefs.20190405-171435-875837.db).
$ sudo kyua debug -k /usr/tests/sys/fs/fusefs/Kyuafile write:Write.append                                                                                             
Note: Google Test filter = Write.append
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from Write
[ RUN      ] Write.append
[  SKIPPED ] Write.append (13 ms)
[----------] 1 test from Write (13 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (13 ms total)
[  PASSED  ] 0 tests.
[  SKIPPED ] 1 test, listed below:
[  SKIPPED ] Write.append
write:Write.append  ->  skipped

Wait... the fact that GTEST_SKIP is not properly emitting any messages is a bug in our (FreeBSD base) version of googletest too. Argh...

ngie-eign · 2019-04-05T18:23:40Z

Hmmm... that didn't work :/...

$ kyua report --results-file=/root/.kyua/store/results.usr_tests_sys_fs_fusefs.20190405-182049-970358.db
kyua: E: Column 'result_reason' is not a string (sqlite db: /root/.kyua/store/results.usr_tests_sys_fs_fusefs.20190405-182049-970358.db).
$ sqlite3 /root/.kyua/store/results.usr_tests_sys_fs_fusefs.20190405-182049-970358.db 
SQLite version 3.27.1 2019-02-08 13:17:39
Enter ".help" for usage hints.
sqlite> SELECT * FROM test_results;
1|skipped|YOU HAVE 1 DISABLED TEST|1554488449986045|1554488449990167
2|skipped|YOU HAVE 1 DISABLED TEST|1554488449992589|1554488449997075
3|skipped|YOU HAVE 1 DISABLED TEST|1554488450000970|1554488450005545
4|skipped||1554488450007688|1554488450022533
5|passed||1554488450029400|1554488450039586
6|passed||1554488450042407|1554488450061054
7|passed||1554488450064575|1554488450077458
8|passed||1554488450080841|1554488450091814
9|passed||1554488450095204|1554488450107807
10|passed||1554488450110648|1554488450123577
11|passed||1554488450126588|1554488450139898
12|skipped||1554488450143172|1554488450153392
13|skipped||1554488450156506|1554488450165748
14|skipped||1554488450169125|1554488450178281
15|skipped||1554488450182709|1554488450193590
16|skipped|YOU HAVE 1 DISABLED TEST|1554488450196618|1554488450201177
17|skipped|YOU HAVE 1 DISABLED TEST|1554488450203280|1554488450207678
18|passed||1554488450209844|1554488450220627

asomers · 2019-04-05T18:26:06Z

As I suspected, the issue is that GTEST_SKIP is being called without a reason:

Shouldn't that be allowed? If you don't supply a reason to GTEST_SKIP, shouldn't it use a sane default, like ""?

ngie-eign · 2019-04-05T18:31:09Z

Shouldn't that be allowed? If you don't supply a reason to GTEST_SKIP, shouldn't it use a sane default, like ""?

That’s what I’m trying to implement as a workaround, but it’s a waste of space on disk.

ngie-eign · 2019-04-05T22:19:24Z

Ok. I'm starting to understand where the issue's coming from:

192 static model::test_result
193 parse_result(sqlite::statement& stmt, const char* type_column,
194              const char* reason_column)
195 {       
196     try {
197         const model::test_result_type type =
198             store::column_test_result_type(stmt, type_column);
199         if (type == model::test_result_passed) {
200             if (stmt.column_type(stmt.column_id(reason_column)) !=
201                 sqlite::type_null)
202                 throw store::integrity_error("Result of type 'passed' has a "
203                                              "non-NULL reason");
204             return model::test_result(type);
205         } else {
206             return model::test_result(type,
207                                       stmt.safe_column_text(reason_column));
208         }

ngie-eign · 2019-04-05T22:26:58Z

Well! It looks like store/write_transaction.cpp:425-426 does the field NULLing for me :D!

409 int64_t
410 store::write_transaction::put_result(const model::test_result& result,
411                                      const int64_t test_case_id,
412                                      const datetime::timestamp& start_time,
413                                      const datetime::timestamp& end_time)
414 {
415     try {
416         sqlite::statement stmt = _pimpl->_db.create_statement(
417             "INSERT INTO test_results (test_case_id, result_type, "
418             "                          result_reason, start_time, "
419             "                          end_time) "
420             "VALUES (:test_case_id, :result_type, :result_reason, "
421             "        :start_time, :end_time)");
422         stmt.bind(":test_case_id", test_case_id);
423 
424         store::bind_test_result_type(stmt, ":result_type", result.type());
425         if (result.reason().empty())
426             stmt.bind(":result_reason", sqlite::null());
427         else
428             stmt.bind(":result_reason", result.reason());

ngie-eign · 2019-04-05T22:36:40Z

And... magic!

$ sudo kyua test -k /usr/tests/sys/fs/fusefs/Kyuafile write
write:AioWrite.DISABLED_aio_write  ->  skipped: YOU HAVE 1 DISABLED TEST  [0.004s]
write:Write.DISABLED_direct_io_evicts_cache  ->  skipped: YOU HAVE 1 DISABLED TEST  [0.004s]
write:Write.DISABLED_mmap  ->  skipped: YOU HAVE 1 DISABLED TEST  [0.004s]
write:Write.append  ->  skipped  [0.012s]
write:Write.append_direct_io  ->  passed  [0.010s]
write:Write.direct_io_short_write  ->  passed  [0.010s]
write:Write.direct_io_short_write_iov  ->  passed  [0.012s]
write:Write.indirect_io_short_write  ->  passed  [0.016s]
write:Write.write  ->  passed  [0.011s]
write:Write.write_large  ->  passed  [0.012s]
write:Write.write_nothing  ->  passed  [0.010s]
write:WriteBack.close  ->  skipped  [0.011s]
write:WriteBack.o_direct  ->  skipped  [0.009s]
write:WriteBack.rmw  ->  skipped  [0.010s]
write:WriteBack.writeback  ->  skipped  [0.013s]
write:WriteThrough.DISABLED_update_file_size  ->  skipped: YOU HAVE 1 DISABLED TEST  [0.007s]
write:WriteThrough.DISABLED_writethrough  ->  skipped: YOU HAVE 1 DISABLED TEST  [0.004s]
write:WriteThrough.pwrite  ->  passed  [0.010s]

Results file id is usr_tests_sys_fs_fusefs.20190405-223507-578754
Results saved to /root/.kyua/store/results.usr_tests_sys_fs_fusefs.20190405-223507-578754.db

18/18 passed (0 failed)
$ kyua report --results-file=/root/.kyua/store/results.usr_tests_sys_fs_fusefs.20190405-223507-578754.db
===> Skipped tests
write:AioWrite.DISABLED_aio_write  ->  skipped: YOU HAVE 1 DISABLED TEST  [0.004s]
write:Write.DISABLED_direct_io_evicts_cache  ->  skipped: YOU HAVE 1 DISABLED TEST  [0.004s]
write:Write.DISABLED_mmap  ->  skipped: YOU HAVE 1 DISABLED TEST  [0.004s]
write:Write.append  ->  skipped  [0.012s]
write:WriteBack.close  ->  skipped  [0.011s]
write:WriteBack.o_direct  ->  skipped  [0.009s]
write:WriteBack.rmw  ->  skipped  [0.010s]
write:WriteBack.writeback  ->  skipped  [0.013s]
write:WriteThrough.DISABLED_update_file_size  ->  skipped: YOU HAVE 1 DISABLED TEST  [0.007s]
write:WriteThrough.DISABLED_writethrough  ->  skipped: YOU HAVE 1 DISABLED TEST  [0.004s]
===> Summary
Results read from /root/.kyua/store/results.usr_tests_sys_fs_fusefs.20190405-223507-578754.db
Test cases: 18 total, 10 skipped, 0 expected failures, 0 broken, 0 failed
Total time: 0.170s

ngie-eign · 2019-04-08T23:55:05Z

@jmmv: I realize this is a large change, but I was wondering if there's a way that I could work with you to get this change reviewed and into the kyua project? I'm asking, because there's a chance that I might get a job in the next few months which will invalidate my individual CLA.

Plus, this change makes it markably easier to run Google Test programs in FreeBSD with Kyua.

ngie-eign · 2019-04-13T03:25:56Z

@jmmv: ping?

ngie-eign · 2019-04-17T00:17:43Z

@jmmv: ping?

ngie-eign · 2019-04-18T17:27:36Z

@jmmv: ping?

ngie-eign · 2019-04-30T02:28:36Z

@jmmv: ping?

asomers · 2019-05-11T15:38:50Z

@jmmv could you please review this change? If this gets merged and released before I finish my fusefs work, it will change how I organize the tests.

jmmv

Apologies for the long delay. This PR is huge and requires a big time and mental investment to get into -- which I had failed to find the time for until now.

The general structure, considering that this is mostly a copy/paste of what other interfaces do, seems fine, so at least that helped with the initial review.

For iterations on this PR, I'd like to ask that you don't amend the commit. Please push iterations as new commits (with garbage descriptions if you like, and without force-pushing) so that GitHub can show incremental diffs. Once we are done, we will just squash all changes into one commit right before merge.

One general question: somewhere you mention that you capture the stdout of the test. What happens with stderr?

engine/googletest.cpp

engine/googletest_result.cpp

This change does the following things: * Leverages utils/cmdline/*, instead of reinventing the wheel. * Consolidates googletest_results tests/setting for clarity. * Eliminates an unnecessary variable/comments. * Removes some duplicative TODOs/fixes TODO comment formatting. * Inlines temporary variables where possible/sensible. * Moves global variables into the anonymous global namespace. * Localizes the variable initialization near its use. * Adds more helpful comments above potentially obfuscated code. * Use list/set initialization instead of using the equivalent unrolled version with `.insert()`/`.push_back()`. * Fix indentation. Signed-off-by: Enji Cooper <yaneurabeya@gmail.com>

jmmv

Yay for GitHub. I can only see a few of the comments when I click on "View changes". Sending replies for those now. Hopefully I can find the other discussions later...

engine/googletest_helpers.cpp

engine/googletest.cpp

engine/googletest_helpers.cpp

engine/googletest.cpp

engine/googletest_list.cpp

engine/googletest_list_test.cpp

engine/googletest_result.cpp

This change does the following things: * Leverages utils/cmdline/*, instead of reinventing the wheel. * Consolidates googletest_results tests/setting for clarity. * Eliminates an unnecessary variable/comments. * Removes some duplicative TODOs/fixes TODO comment formatting. * Inlines temporary variables where possible/sensible. * Moves global variables into the anonymous global namespace. * Localizes the variable initialization near its use. * Adds more helpful comments above potentially obfuscated code. * Use list/set initialization instead of using the equivalent unrolled version with `.insert()`/`.push_back()`. * Fix indentation. Signed-off-by: Enji Cooper <yaneurabeya@gmail.com>

* Use initializer with std::map instead of initializing by hand. * Fix method definition indentation. * const poison some variables and inline where possible, eliminating need for unnecessary intermediate values. * Eliminate obfuscated proposed solution in `engine::googletest_result::apply` and deduplicate code by using a lambda to check the failed vs !failed case when checking the exit status. * Remove superfluous parentheses. Signed-off-by: Enji Cooper <yaneurabeya@gmail.com>

ngie-eign · 2019-08-26T20:49:20Z

@jmmv: ping..?

ngie-eign · 2019-09-24T22:51:46Z

@emaste: given the lack of feedback in this PR, I think it's time to consider officially forking atf/kyua/lutok into a FreeBSD-org managed repo and install the ports from that repo. I'll reach out to you soon about this.

This change adds fine grained execution of Google Test test programs. First, the Google Test test is executed with `--gtest_list_tests`. Next, based on the output from `--gtest_list_tests`, the testcases are run individually. The output from each unique testcase is based on the standard output it provides, per the test output protocol defined in the GoogleTest docs on github [1], [2], and instrumented via various demo programs I created, which can be found on GitHub [here](https://github.com/ngie-eign/scratch/tree/master/programming/c%2B%2B/gtest). This support is a very rough cut to provide an initial working integration effort. There're additional improvements that can be made by leveraging either the JSON or XML structured output format, instead of scraping standard output using beginning and ending sentinels to search for regular expressions. In order to do that though without reinventing the wheel, Kyua would need to rely on an external JSON or XML library. This test interface supports fine grained execution of test programs like the ATF test interface, but matches behavior of plain/TAP interfaces by instead supporting metadata passing via `$TEST_ENV_` prefixed environment variables. This support adds additional tests for verifying pass, fail, skip (will be available in version 1.9.0 and is available in FreeBSD base's version of Google Test), and disabled result determination, using mock output and a mock test program (`engine/googletest_helpers`), for parity with other test formats (ATF, plain, TAP). The helper test program purposely avoids relying on `getopt_long*` for portability reasons, and the GoogleTest test infrastructure, in order to limit Kyua's dependencies. As part of this change, `store/read_transaction.cpp` needed to support optional reasons provided with skip results. While it's bad form to omit test results with tests, providing a reason is optional with Google Test, and unfortunately not all portions of the test framework output a reason when `GTEST_SKIP()` is called. See the issue in [3] for one such example issue when `GTEST_SKIP()` is called from SetUp test fixtures. 1. https://github.com/google/googletest/blob/master/googletest/docs/primer.md 2. https://github.com/google/googletest/blob/master/googletest/docs/advanced.md 3. google/googletest#2208 Signed-off-by: Enji Cooper <yaneurabeya@gmail.com>

This change does the following things: * Leverages utils/cmdline/*, instead of reinventing the wheel. * Consolidates googletest_results tests/setting for clarity. * Eliminates an unnecessary variable/comments. * Removes some duplicative TODOs/fixes TODO comment formatting. * Inlines temporary variables where possible/sensible. * Moves global variables into the anonymous global namespace. * Localizes the variable initialization near its use. * Adds more helpful comments above potentially obfuscated code. * Use list/set initialization instead of using the equivalent unrolled version with `.insert()`/`.push_back()`. * Fix indentation. Signed-off-by: Enji Cooper <yaneurabeya@gmail.com>

Document parameters moved into the anonymous namespace in 6e171b0 . Signed-off-by: Enji Cooper <yaneurabeya@gmail.com>

* Use initializer with std::map instead of initializing by hand. * Fix method definition indentation. * const poison some variables and inline where possible, eliminating need for unnecessary intermediate values. * Eliminate obfuscated proposed solution in `engine::googletest_result::apply` and deduplicate code by using a lambda to check the failed vs !failed case when checking the exit status. * Remove superfluous parentheses. Signed-off-by: Enji Cooper <yaneurabeya@gmail.com>

This allows kyua to function in a backwards compatible manner, allowing previous versions to read databases from newer versions. Signed-off-by: Enji Cooper <yaneurabeya@gmail.com>

The version of Doxygen used by Travis CI thinks that the global `std::regex` variables need to have a `\return` type annotation (they don't). Make them local to the function again and add a TODO. Signed-off-by: Enji Cooper <yaneurabeya@gmail.com>

googlebot added the cla: yes label Apr 3, 2019

ngie-eign force-pushed the add-googletest-support branch 3 times, most recently from 47d9d18 to 06f5045 Compare April 3, 2019 02:34

ngie-eign changed the title ~~Add support for executing googletest test programs~~ Add Google Test support Apr 3, 2019

ngie-eign force-pushed the add-googletest-support branch 4 times, most recently from faf4eac to faea701 Compare April 3, 2019 02:40

ngie-eign force-pushed the add-googletest-support branch from faea701 to 6e6a8b2 Compare April 3, 2019 03:55

ngie-eign force-pushed the add-googletest-support branch 3 times, most recently from 633e3ee to ff52f04 Compare April 6, 2019 03:48

jmmv reviewed May 13, 2019

View reviewed changes

jmmv reviewed May 15, 2019

View reviewed changes

ngie-eign force-pushed the add-googletest-support branch from 38b7189 to fa43d07 Compare May 20, 2019 04:43

ngie-eign force-pushed the add-googletest-support branch from fa43d07 to 07a6708 Compare May 20, 2019 04:45

lwhsu mentioned this pull request Jan 17, 2024

Add GoogleTest support freebsd/kyua-fork#11

Closed

ngie-eign added 6 commits May 9, 2024 02:19

Fix "make check-api-docs" with Travis CI

3e53074

Document parameters moved into the anonymous namespace in 6e171b0 . Signed-off-by: Enji Cooper <yaneurabeya@gmail.com>

Provide a bogus result (a newline) when a skipped result has no context

85ee53f

This allows kyua to function in a backwards compatible manner, allowing previous versions to read databases from newer versions. Signed-off-by: Enji Cooper <yaneurabeya@gmail.com>

Workaround a Doxygen bug

b99bf61

The version of Doxygen used by Travis CI thinks that the global `std::regex` variables need to have a `\return` type annotation (they don't). Make them local to the function again and add a TODO. Signed-off-by: Enji Cooper <yaneurabeya@gmail.com>

ngie-eign force-pushed the add-googletest-support branch from 1f9eaea to b99bf61 Compare May 9, 2024 10:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Google Test support #203

Add Google Test support #203

ngie-eign commented Apr 3, 2019 •

edited

ngie-eign commented Apr 3, 2019

asomers commented Apr 3, 2019

ngie-eign commented Apr 3, 2019

asomers commented Apr 3, 2019

ngie-eign commented Apr 3, 2019

ngie-eign commented Apr 5, 2019 •

edited

ngie-eign commented Apr 5, 2019

asomers commented Apr 5, 2019

ngie-eign commented Apr 5, 2019

ngie-eign commented Apr 5, 2019

ngie-eign commented Apr 5, 2019 •

edited

ngie-eign commented Apr 5, 2019

ngie-eign commented Apr 8, 2019

ngie-eign commented Apr 13, 2019

ngie-eign commented Apr 17, 2019

ngie-eign commented Apr 18, 2019

ngie-eign commented Apr 30, 2019

asomers commented May 11, 2019

jmmv left a comment

jmmv left a comment

ngie-eign commented Aug 26, 2019

ngie-eign commented Sep 24, 2019 •

edited

Add Google Test support #203

Are you sure you want to change the base?

Add Google Test support #203

Conversation

ngie-eign commented Apr 3, 2019 • edited

ngie-eign commented Apr 3, 2019

asomers commented Apr 3, 2019

ngie-eign commented Apr 3, 2019

asomers commented Apr 3, 2019

ngie-eign commented Apr 3, 2019

ngie-eign commented Apr 5, 2019 • edited

ngie-eign commented Apr 5, 2019

asomers commented Apr 5, 2019

ngie-eign commented Apr 5, 2019

ngie-eign commented Apr 5, 2019

ngie-eign commented Apr 5, 2019 • edited

ngie-eign commented Apr 5, 2019

ngie-eign commented Apr 8, 2019

ngie-eign commented Apr 13, 2019

ngie-eign commented Apr 17, 2019

ngie-eign commented Apr 18, 2019

ngie-eign commented Apr 30, 2019

asomers commented May 11, 2019

jmmv left a comment

Choose a reason for hiding this comment

jmmv left a comment

Choose a reason for hiding this comment

ngie-eign commented Aug 26, 2019

ngie-eign commented Sep 24, 2019 • edited

ngie-eign commented Apr 3, 2019 •

edited

ngie-eign commented Apr 5, 2019 •

edited

ngie-eign commented Apr 5, 2019 •

edited

ngie-eign commented Sep 24, 2019 •

edited