Previous email was sent before being finished for mistake. Rest contents as below,
Since sanity checking still have some concerns, I'd hold on related code change. In the future, we could add some common check functions in TestPrepare if necessary, and move the code to the lib path if necessary. Test developers could call related common check functions and fail cases earlier if necessary. 2016-07-20 10:23 GMT+08:00 Paul Guo <[email protected]>: > I totally agree that test case results should be tristate (not just OK and > FAIL). > Unfortually Goggletest does not return a runtime SKIPPED similar results. > So I guess we have to fail for those "skipped" cases. > > Note Goggletest could skip running or disable cases, but that is not what > we expect. > > In goggliest, case is disabled by explicitly adding DISABLED_ before case > name, e.g. > > TEST(FooTest, DISABLED_DoesAbc) { ... } > https://github.com/google/googletest/blob/master/googletest/docs/V1_7_AdvancedGuide.md#temporarily-disabling-tests > > or use gtest_filter option in command line to skip some cases. > > I introduced a new class TestPrepare in a git code change (Not motivated > for this thread). > > HAWQ-925. Set default locale, timezone & datastyle before running sql > command/file > > Since sanity checking still have some concerns, > <https://github.com/apache/incubator-hawq/pull/802/files> > > > > 2016-07-12 15:58 GMT+08:00 Lili Ma <[email protected]>: > >> Agree with @Jiali on the "SKIPPED" solution, but I think we need do some >> emphasis for the skipped test cases to let the user know. The aim is to >> avoid the user who run the test thinks all the tests have run >> successfully, >> including the skipped tests. >> >> 2016-07-12 14:22 GMT+08:00 Jiali Yao <[email protected]>: >> >> > For the test case checking, I think it should report "SKIPPED" instead >> of >> > ERROR. >> > The test case should check whether this feature is supported or not. If >> > supported, run the case; otherwise skipped it. >> > Agree on that we should add it in common lib. >> > >> > On the other topic, I think source greenplum_path.sh is must. It is env >> > related. >> > >> > Thanks >> > >> > Jiali >> > >> > >> > >> > On Tue, Jul 12, 2016 at 2:19 PM, Lei Chang <[email protected]> >> wrote: >> > >> > > I think the better way is to let test cases run under some conditions. >> > > >> > > for example, pl/python is optional, if user did not run configure with >> > > pl/python option, the test about pl/python should not run. >> > > >> > > Cheers >> > > Lei >> > > >> > > >> > > >> > > On Tue, Jul 12, 2016 at 2:15 PM, Ivan Weng <[email protected]> wrote: >> > > >> > > > Agree with Hong. Test case should check its environment needed. If >> the >> > > > check failed, it should terminate the execution and report the >> error. >> > > > >> > > > On Tue, Jul 12, 2016 at 2:04 PM, Hong Wu <[email protected]> >> > wrote: >> > > > >> > > > > It is user/developer themselves that should take care. Say, if you >> > > write >> > > > a >> > > > > test case which is related to plpython, why don't you configure >> HAWQ >> > > with >> > > > > "--with-python" option? We should write a README for feature-test >> > that >> > > > > guides user to run this tests. For example, tell them sourcing >> > > > > "greenplum.sh" before running tests. >> > > > > >> > > > > Consequently, I think add such sanity-check is a little bit of >> > > > > over-engineering which will bring extra problems and complexities. >> > > > > >> > > > > Best >> > > > > xunzhang >> > > > > >> > > > > 2016-07-12 13:47 GMT+08:00 Paul Guo <[email protected]>: >> > > > > >> > > > > > I have >1 times to encounter some feature test failures due to >> > > reported >> > > > > > missing stuffs. >> > > > > > >> > > > > > e.g. >> > > > > > >> > > > > > 1. I did not have pl/python installed in my hawq build so >> > > > > > UDF/sql/function_set_returning.sql fails to "create language >> > > > > plpythonu" >> > > > > > This makes this case fails. >> > > > > > >> > > > > > 2. Sometimes I forgot to source a greenplum.sh, then all cases >> run >> > > > > > with failures due to missing psql. >> > > > > > >> > > > > > We seem to be able to improve. >> > > > > > >> > > > > > 1) Sanity-check some file existence in common code, e.g. >> > > > > > psql, gpdiff.pl, >> > > > > > >> > > > > > 2) Some cases could do sanity-check in their own test >> constructor >> > > > > > functions, >> > > > > > e.g. if the case uses the extension plpython, the test case >> > > should >> > > > > > check it itself. >> > > > > > >> > > > > > More thoughts? >> > > > > > >> > > > > >> > > > >> > > >> > >> > >
