You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hawq.apache.org by amyrazz44 <gi...@git.apache.org> on 2016/11/10 02:02:25 UTC

[GitHub] incubator-hawq pull request #1007: HAWQ-1148. Update gtest-parallel to make ...

GitHub user amyrazz44 opened a pull request:

    https://github.com/apache/incubator-hawq/pull/1007

    HAWQ-1148. Update gtest-parallel to make sure test case can run in bo\u2026

    \u2026th parallel way and serial way
    
    Signed-off-by: wengyanqing <we...@gmail.com>

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/amyrazz44/incubator-hawq scheduleTest

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-hawq/pull/1007.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1007
    
----
commit 1a5b421917efd9e3f89093647a5ce365e4b0c852
Author: amyrazz44 <ab...@pivotal.io>
Date:   2016-11-08T08:03:55Z

    HAWQ-1148. Update gtest-parallel to make sure test case can run in both parallel way and serial way
    
    Signed-off-by: wengyanqing <we...@gmail.com>

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1007: HAWQ-1148. Update gtest-parallel to make ...

Posted by amyrazz44 <gi...@git.apache.org>.
Github user amyrazz44 commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1007#discussion_r87372883
  
    --- Diff: src/test/feature/schedule.txt ---
    @@ -0,0 +1,6 @@
    +#PARALLEL=* are the parallel tests to run, optional but should not be empty
    +#SERIAL=* are the serial tests to run, optional but should not be empty
    +#you can have several PARALLEL or SRRIAL
    +
    +PARALLEL=TestErrorTable.*:TestExternalTable.*:TestPreparedStatement.*:TestUDF.*:TestAOSnappy.*:TestAlterOwner.*:TestAlterTable.*:TestCreateTable.*:TestGuc.*:TestType.*:TestDatabase.*:TestParquet.*:TestPartition.*:TestSubplan.*:TestAggregate.*:TestCreateTypeComposite.*:TestGpDistRandom.*:TestInformationSchema.*:TestQueryInsert.*:TestQueryNestedCaseNull.*:TestQueryPolymorphism.*:TestQueryPortal.*:TestQueryPrepare.*:TestRowTypes.*:TestQuerySequence.*:TestCommonLib.*:TestToast.*:TestTransaction.*:TestCommand.*:TestCopy.*:TestExternalTable.TestExternalTableAll
    --- End diff --
    
    @paul-guo- If users want to add new cases temporary , schedule file contains the test cases in hawq-ci will be easily changed to run test case.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1007: HAWQ-1148. Update gtest-parallel to make ...

Posted by amyrazz44 <gi...@git.apache.org>.
Github user amyrazz44 commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1007#discussion_r87325037
  
    --- Diff: src/test/feature/schedule.txt ---
    @@ -0,0 +1,6 @@
    +#PARALLEL=* are the parallel tests to run, optional but should not be empty
    +#SERIAL=* are the serial tests to run, optional but should not be empty
    +#you can have several PARALLEL or SRRIAL
    +
    +PARALLEL=TestErrorTable.*:TestExternalTable.*:TestPreparedStatement.*:TestUDF.*:TestAOSnappy.*:TestAlterOwner.*:TestAlterTable.*:TestCreateTable.*:TestGuc.*:TestType.*:TestDatabase.*:TestParquet.*:TestPartition.*:TestSubplan.*:TestAggregate.*:TestCreateTypeComposite.*:TestGpDistRandom.*:TestInformationSchema.*:TestQueryInsert.*:TestQueryNestedCaseNull.*:TestQueryPolymorphism.*:TestQueryPortal.*:TestQueryPrepare.*:TestRowTypes.*:TestQuerySequence.*:TestCommonLib.*:TestToast.*:TestTransaction.*:TestCommand.*:TestCopy.*:TestExternalTable.TestExternalTableAll
    --- End diff --
    
    Will refine this, thank you


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1007: HAWQ-1148. Update gtest-parallel to make ...

Posted by amyrazz44 <gi...@git.apache.org>.
Github user amyrazz44 commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1007#discussion_r87322648
  
    --- Diff: src/test/feature/gtest-parallel ---
    @@ -234,6 +234,71 @@ class TestTimes(object):
         except IOError:
           pass  # ignore errors---saving the times isn't that important
     
    +
    +def get_schedule(schedule_file):
    +  "read schedule file from --gtest_schedule option"
    +  try:
    +    file = open(schedule_file, 'r')
    +  except (EOFError, IOError):
    +    sys.exit("Read file error")
    +  
    +  filter_test = []
    +  ptest = []
    +  stest = []
    +
    +  for line in file.readlines():
    +    if not line.strip():
    +      continue
    +    if line[0] == '#':
    +      continue
    +
    +    testline = line.split('=')
    +    if len(testline) != 2 or (not testline[1].strip()):
    +      sys.exit("format error in schedule file")
    +    
    +    (key, value) = testline
    +    if key == "PARALLEL":
    +      ptest.append(value) 
    +    elif key == "SERIAL":
    +      stest.append(value)
    +    else:
    +      sys.exit("format error in schedule file")
    +
    +  filter_test = ptest + stest	
    +  file.close()
    +  return len(ptest), filter_test
    +
    +
    +def do_gtest_filter(list_command, command, op_filter):
    +    "get tests by --gtest_filter and --gtest_list_tests"
    +    list_command += ['--gtest_filter=' + op_filter]
    +    try:
    +      test_list = subprocess.Popen(list_command + ['--gtest_list_tests'],
    +                                 stdout=subprocess.PIPE).communicate()[0]
    +    except OSError as e:
    +      sys.exit("%s: %s" % (test_binary, str(e)))
    +
    +    command += additional_args
    +    tests = []
    +    test_group = ''
    +    for line in test_list.split('\n'):
    +      if not line.strip():
    --- End diff --
    
    To deal with the blank line.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1007: HAWQ-1148. Update gtest-parallel to make ...

Posted by amyrazz44 <gi...@git.apache.org>.
Github user amyrazz44 commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1007#discussion_r87322394
  
    --- Diff: src/test/feature/gtest-parallel ---
    @@ -170,7 +170,7 @@ class FilterFormat:
           self.out.permanent_line("FAILED TESTS (%d/%d):"
                                   % (len(self.failures), self.total_tests))
           for (binary, test) in self.failures:
    -        self.out.permanent_line(" " + binary + ": " + test)
    +        self.out.permanent_line(" " + binary + ": " + test + " [" + test_map[test] + "]")
    --- End diff --
    
    Just to keep consistence with the origin code.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1007: HAWQ-1148. Update gtest-parallel to make ...

Posted by amyrazz44 <gi...@git.apache.org>.
Github user amyrazz44 commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1007#discussion_r87322273
  
    --- Diff: src/test/feature/gtest-parallel ---
    @@ -234,6 +234,71 @@ class TestTimes(object):
         except IOError:
           pass  # ignore errors---saving the times isn't that important
     
    +
    +def get_schedule(schedule_file):
    +  "read schedule file from --gtest_schedule option"
    +  try:
    +    file = open(schedule_file, 'r')
    +  except (EOFError, IOError):
    +    sys.exit("Read file error")
    +  
    +  filter_test = []
    +  ptest = []
    +  stest = []
    +
    +  for line in file.readlines():
    --- End diff --
    
    Will consider this, thank you.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1007: HAWQ-1148. Update gtest-parallel to make ...

Posted by paul-guo- <gi...@git.apache.org>.
Github user paul-guo- commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1007#discussion_r87320349
  
    --- Diff: src/test/feature/README.md ---
    @@ -15,8 +15,7 @@ Before building the code of feature tests part, just make sure your compiler sup
     1. Make sure HAWQ is running correctly. If not, `init` or `start` HAWQ at first. Note please don't set locale related arguments for hawq init.
     2. Load environment configuration by running `source $INSTALL_PREFIX/greenplum_path.sh`.
     3. Load hdfs configuration. For example, `export HADOOP_HOME=/Users/wuhong/hadoop-2.7.2 && export PATH=${PATH}:${HADOOP_HOME}/bin`. Since some test cases need `hdfs` and `hadoop` command, just ensure these commands work before running. Otherwise you will get failure.
    -4. Run the cases with`./parallel-run-feature-test.sh 8 ./feature-test`(in this case 8 threads in parallel), you could use `--gtest_filter` option to filter test cases(both positive and negative patterns are supported). Please see more options by running `./feature-test --help`.
    -5. As for now, there are several cases which in the sequence.txt file can not parallel run. Run the cases with `./parallel-run-feature-test.sh 8 ./feature-test --gtest_filter=-`cat ./sequence.txt``.
    +4. Run the cases with`./parallel-run-feature-test.sh 8 ./feature-test`(in this case 8 threads in parallel), you could use `--gtest_filter` option to filter test cases(both positive and negative patterns are supported). You can also run cases with `--gtest_schedule` (eg. --gtest_schedule=./schedule.txt) if you want to run cases in both parallel way and serial way. Please see more options by running `./feature-test --help`.
    --- End diff --
    
    1) use -> add the?
    2) Maybe add a simple example for --gtest_filter.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1007: HAWQ-1148. Update gtest-parallel to make ...

Posted by xunzhang <gi...@git.apache.org>.
Github user xunzhang commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1007#discussion_r87318672
  
    --- Diff: src/test/feature/gtest-parallel ---
    @@ -234,6 +234,71 @@ class TestTimes(object):
         except IOError:
           pass  # ignore errors---saving the times isn't that important
     
    +
    +def get_schedule(schedule_file):
    +  "read schedule file from --gtest_schedule option"
    +  try:
    +    file = open(schedule_file, 'r')
    +  except (EOFError, IOError):
    +    sys.exit("Read file error")
    +  
    +  filter_test = []
    +  ptest = []
    +  stest = []
    +
    +  for line in file.readlines():
    +    if not line.strip():
    +      continue
    +    if line[0] == '#':
    +      continue
    +
    +    testline = line.split('=')
    +    if len(testline) != 2 or (not testline[1].strip()):
    +      sys.exit("format error in schedule file")
    +    
    +    (key, value) = testline
    +    if key == "PARALLEL":
    +      ptest.append(value) 
    +    elif key == "SERIAL":
    +      stest.append(value)
    +    else:
    +      sys.exit("format error in schedule file")
    +
    +  filter_test = ptest + stest	
    +  file.close()
    +  return len(ptest), filter_test
    +
    +
    +def do_gtest_filter(list_command, command, op_filter):
    +    "get tests by --gtest_filter and --gtest_list_tests"
    +    list_command += ['--gtest_filter=' + op_filter]
    +    try:
    +      test_list = subprocess.Popen(list_command + ['--gtest_list_tests'],
    +                                 stdout=subprocess.PIPE).communicate()[0]
    +    except OSError as e:
    +      sys.exit("%s: %s" % (test_binary, str(e)))
    +
    +    command += additional_args
    +    tests = []
    +    test_group = ''
    +    for line in test_list.split('\n'):
    +      if not line.strip():
    --- End diff --
    
    What is the meaning of this line?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1007: HAWQ-1148. Update gtest-parallel to make ...

Posted by xunzhang <gi...@git.apache.org>.
Github user xunzhang commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1007#discussion_r87318413
  
    --- Diff: src/test/feature/gtest-parallel ---
    @@ -234,6 +234,71 @@ class TestTimes(object):
         except IOError:
           pass  # ignore errors---saving the times isn't that important
     
    +
    +def get_schedule(schedule_file):
    +  "read schedule file from --gtest_schedule option"
    +  try:
    +    file = open(schedule_file, 'r')
    +  except (EOFError, IOError):
    +    sys.exit("Read file error")
    +  
    +  filter_test = []
    +  ptest = []
    +  stest = []
    +
    +  for line in file.readlines():
    --- End diff --
    
    Actually I do not like the for-loop her because I think it is a little tricky. If you need a key-value dict information, could we change the file with json format?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq issue #1007: HAWQ-1148. Update gtest-parallel to make sure te...

Posted by xunzhang <gi...@git.apache.org>.
Github user xunzhang commented on the issue:

    https://github.com/apache/incubator-hawq/pull/1007
  
    LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1007: HAWQ-1148. Update gtest-parallel to make ...

Posted by xunzhang <gi...@git.apache.org>.
Github user xunzhang commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1007#discussion_r87318874
  
    --- Diff: src/test/feature/gtest-parallel ---
    @@ -285,41 +354,42 @@ else:
     save_file = os.path.join(os.path.expanduser("~"), ".gtest-parallel-times")
     times = TestTimes(save_file)
     tests = []
    +#pull all the tests into test_map dict, in order to mark the failed test in FAILED log 
    +test_map = {} 
    +# mark the end of paralell test id by parallel_id
    +parallel_id = 0
    +
     for test_binary in binaries:
       command = [test_binary]
       if options.gtest_also_run_disabled_tests:
         command += ['--gtest_also_run_disabled_tests']
    -
       list_command = list(command)
    +  
    +  if options.gtest_schedule != '' and options.gtest_filter != '':
    +    sys.exit("Option input failure : gtest_schedule and gtest_filter can not use in the same time: \n")
    +    
    +  if options.gtest_schedule != '':
    +    (pcount, filter_test) = get_schedule(options.gtest_schedule)
    +    for i in range(0, len(filter_test)):
    --- End diff --
    
    It is like C-style, not pythonic. Use enumerate instead to make the code simple. You can implement this logic using less than 10 lines of code.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1007: HAWQ-1148. Update gtest-parallel to make ...

Posted by xunzhang <gi...@git.apache.org>.
Github user xunzhang commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1007#discussion_r87317655
  
    --- Diff: src/test/feature/gtest-parallel ---
    @@ -170,7 +170,7 @@ class FilterFormat:
           self.out.permanent_line("FAILED TESTS (%d/%d):"
                                   % (len(self.failures), self.total_tests))
           for (binary, test) in self.failures:
    -        self.out.permanent_line(" " + binary + ": " + test)
    +        self.out.permanent_line(" " + binary + ": " + test + " [" + test_map[test] + "]")
    --- End diff --
    
    I suggest you to use format string for readability and correctness. For example, use `" %s: %s [ %s ]" % (binary, test, test_map[test])` here.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1007: HAWQ-1148. Update gtest-parallel to make ...

Posted by xunzhang <gi...@git.apache.org>.
Github user xunzhang commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1007#discussion_r87318201
  
    --- Diff: src/test/feature/schedule.txt ---
    @@ -0,0 +1,6 @@
    +#PARALLEL=* are the parallel tests to run, optional but should not be empty
    +#SERIAL=* are the serial tests to run, optional but should not be empty
    +#you can have several PARALLEL or SRRIAL
    +
    +PARALLEL=TestErrorTable.*:TestExternalTable.*:TestPreparedStatement.*:TestUDF.*:TestAOSnappy.*:TestAlterOwner.*:TestAlterTable.*:TestCreateTable.*:TestGuc.*:TestType.*:TestDatabase.*:TestParquet.*:TestPartition.*:TestSubplan.*:TestAggregate.*:TestCreateTypeComposite.*:TestGpDistRandom.*:TestInformationSchema.*:TestQueryInsert.*:TestQueryNestedCaseNull.*:TestQueryPolymorphism.*:TestQueryPortal.*:TestQueryPrepare.*:TestRowTypes.*:TestQuerySequence.*:TestCommonLib.*:TestToast.*:TestTransaction.*:TestCommand.*:TestCopy.*:TestExternalTable.TestExternalTableAll
    --- End diff --
    
    Should we add some checks in the python script above to ensure the valid input schedule?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1007: HAWQ-1148. Update gtest-parallel to make ...

Posted by amyrazz44 <gi...@git.apache.org>.
Github user amyrazz44 commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1007#discussion_r87322601
  
    --- Diff: src/test/feature/gtest-parallel ---
    @@ -234,6 +234,71 @@ class TestTimes(object):
         except IOError:
           pass  # ignore errors---saving the times isn't that important
     
    +
    +def get_schedule(schedule_file):
    +  "read schedule file from --gtest_schedule option"
    +  try:
    +    file = open(schedule_file, 'r')
    +  except (EOFError, IOError):
    +    sys.exit("Read file error")
    +  
    +  filter_test = []
    +  ptest = []
    +  stest = []
    --- End diff --
    
    Will consider , thank you


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1007: HAWQ-1148. Update gtest-parallel to make ...

Posted by xunzhang <gi...@git.apache.org>.
Github user xunzhang commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1007#discussion_r87318470
  
    --- Diff: src/test/feature/gtest-parallel ---
    @@ -234,6 +234,71 @@ class TestTimes(object):
         except IOError:
           pass  # ignore errors---saving the times isn't that important
     
    +
    +def get_schedule(schedule_file):
    +  "read schedule file from --gtest_schedule option"
    +  try:
    +    file = open(schedule_file, 'r')
    +  except (EOFError, IOError):
    +    sys.exit("Read file error")
    +  
    +  filter_test = []
    +  ptest = []
    +  stest = []
    +
    +  for line in file.readlines():
    --- End diff --
    
    I do not understand, is it just one line of total file content here?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1007: HAWQ-1148. Update gtest-parallel to make ...

Posted by paul-guo- <gi...@git.apache.org>.
Github user paul-guo- commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1007#discussion_r87320997
  
    --- Diff: src/test/feature/schedule.txt ---
    @@ -0,0 +1,6 @@
    +#PARALLEL=* are the parallel tests to run, optional but should not be empty
    +#SERIAL=* are the serial tests to run, optional but should not be empty
    +#you can have several PARALLEL or SRRIAL
    +
    +PARALLEL=TestErrorTable.*:TestExternalTable.*:TestPreparedStatement.*:TestUDF.*:TestAOSnappy.*:TestAlterOwner.*:TestAlterTable.*:TestCreateTable.*:TestGuc.*:TestType.*:TestDatabase.*:TestParquet.*:TestPartition.*:TestSubplan.*:TestAggregate.*:TestCreateTypeComposite.*:TestGpDistRandom.*:TestInformationSchema.*:TestQueryInsert.*:TestQueryNestedCaseNull.*:TestQueryPolymorphism.*:TestQueryPortal.*:TestQueryPrepare.*:TestRowTypes.*:TestQuerySequence.*:TestCommonLib.*:TestToast.*:TestTransaction.*:TestCommand.*:TestCopy.*:TestExternalTable.TestExternalTableAll
    --- End diff --
    
    Just curious. Does that means if dev adds a test class, they must modify this file also. Maybe we should try to make the default cases run in parallel by default.
    
    By the way, all of these in one line is really not human readable and is fragile. I'd suggest list them one line by line. You could use a custom format or yaml whatever format?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq issue #1007: HAWQ-1148. Update gtest-parallel to make sure te...

Posted by xunzhang <gi...@git.apache.org>.
Github user xunzhang commented on the issue:

    https://github.com/apache/incubator-hawq/pull/1007
  
    Please close this pull request after merge. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1007: HAWQ-1148. Update gtest-parallel to make ...

Posted by huor <gi...@git.apache.org>.
Github user huor commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1007#discussion_r87531052
  
    --- Diff: src/test/feature/schedule.txt ---
    @@ -0,0 +1,6 @@
    +#PARALLEL=* are the parallel tests to run, optional but should not be empty
    +#SERIAL=* are the serial tests to run, optional but should not be empty
    +#you can have several PARALLEL or SRRIAL
    +
    +PARALLEL=TestErrorTable.*:TestExternalTable.*:TestPreparedStatement.*:TestUDF.*:TestAOSnappy.*:TestAlterOwner.*:TestAlterTable.*:TestCreateTable.*:TestGuc.*:TestType.*:TestDatabase.*:TestParquet.*:TestPartition.*:TestSubplan.*:TestAggregate.*:TestCreateTypeComposite.*:TestGpDistRandom.*:TestInformationSchema.*:TestQueryInsert.*:TestQueryNestedCaseNull.*:TestQueryPolymorphism.*:TestQueryPortal.*:TestQueryPrepare.*:TestRowTypes.*:TestQuerySequence.*:TestCommonLib.*:TestToast.*:TestTransaction.*:TestCommand.*:TestCopy.*:TestExternalTable.TestExternalTableAll
    --- End diff --
    
    The name of the schedule can be more specific, i.e, scheduke_sanity, schedule_full_regression, etc.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq issue #1007: HAWQ-1148. Update gtest-parallel to make sure te...

Posted by amyrazz44 <gi...@git.apache.org>.
Github user amyrazz44 commented on the issue:

    https://github.com/apache/incubator-hawq/pull/1007
  
    @paul-guo- @xunzhang  pls review this pr , thank you.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1007: HAWQ-1148. Update gtest-parallel to make ...

Posted by xunzhang <gi...@git.apache.org>.
Github user xunzhang commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1007#discussion_r87317799
  
    --- Diff: src/test/feature/gtest-parallel ---
    @@ -234,6 +234,71 @@ class TestTimes(object):
         except IOError:
           pass  # ignore errors---saving the times isn't that important
     
    +
    +def get_schedule(schedule_file):
    +  "read schedule file from --gtest_schedule option"
    +  try:
    +    file = open(schedule_file, 'r')
    +  except (EOFError, IOError):
    +    sys.exit("Read file error")
    +  
    +  filter_test = []
    +  ptest = []
    +  stest = []
    --- End diff --
    
    `filter_test, ptest, stest = [], [], []` is better in Python.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1007: HAWQ-1148. Update gtest-parallel to make ...

Posted by amyrazz44 <gi...@git.apache.org>.
Github user amyrazz44 commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1007#discussion_r87322333
  
    --- Diff: src/test/feature/gtest-parallel ---
    @@ -234,6 +234,71 @@ class TestTimes(object):
         except IOError:
           pass  # ignore errors---saving the times isn't that important
     
    +
    +def get_schedule(schedule_file):
    +  "read schedule file from --gtest_schedule option"
    +  try:
    +    file = open(schedule_file, 'r')
    +  except (EOFError, IOError):
    +    sys.exit("Read file error")
    +  
    +  filter_test = []
    +  ptest = []
    +  stest = []
    +
    +  for line in file.readlines():
    --- End diff --
    
    One line of PARALLEL test case or SERIAL test case. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1007: HAWQ-1148. Update gtest-parallel to make ...

Posted by huor <gi...@git.apache.org>.
Github user huor commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1007#discussion_r87530913
  
    --- Diff: src/test/feature/schedule.txt ---
    @@ -0,0 +1,6 @@
    +#PARALLEL=* are the parallel tests to run, optional but should not be empty
    +#SERIAL=* are the serial tests to run, optional but should not be empty
    +#you can have several PARALLEL or SRRIAL
    +
    +PARALLEL=TestErrorTable.*:TestExternalTable.*:TestPreparedStatement.*:TestUDF.*:TestAOSnappy.*:TestAlterOwner.*:TestAlterTable.*:TestCreateTable.*:TestGuc.*:TestType.*:TestDatabase.*:TestParquet.*:TestPartition.*:TestSubplan.*:TestAggregate.*:TestCreateTypeComposite.*:TestGpDistRandom.*:TestInformationSchema.*:TestQueryInsert.*:TestQueryNestedCaseNull.*:TestQueryPolymorphism.*:TestQueryPortal.*:TestQueryPrepare.*:TestRowTypes.*:TestQuerySequence.*:TestCommonLib.*:TestToast.*:TestTransaction.*:TestCommand.*:TestCopy.*:TestExternalTable.TestExternalTableAll
    --- End diff --
    
    It would be good to use yaml for the schedule which is more human readable.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1007: HAWQ-1148. Update gtest-parallel to make ...

Posted by xunzhang <gi...@git.apache.org>.
Github user xunzhang commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1007#discussion_r87323234
  
    --- Diff: src/test/feature/gtest-parallel ---
    @@ -234,6 +234,71 @@ class TestTimes(object):
         except IOError:
           pass  # ignore errors---saving the times isn't that important
     
    +
    +def get_schedule(schedule_file):
    +  "read schedule file from --gtest_schedule option"
    +  try:
    +    file = open(schedule_file, 'r')
    +  except (EOFError, IOError):
    +    sys.exit("Read file error")
    +  
    +  filter_test = []
    +  ptest = []
    +  stest = []
    +
    +  for line in file.readlines():
    +    if not line.strip():
    +      continue
    +    if line[0] == '#':
    +      continue
    +
    +    testline = line.split('=')
    +    if len(testline) != 2 or (not testline[1].strip()):
    +      sys.exit("format error in schedule file")
    +    
    +    (key, value) = testline
    +    if key == "PARALLEL":
    +      ptest.append(value) 
    +    elif key == "SERIAL":
    +      stest.append(value)
    +    else:
    +      sys.exit("format error in schedule file")
    +
    +  filter_test = ptest + stest	
    +  file.close()
    +  return len(ptest), filter_test
    +
    +
    +def do_gtest_filter(list_command, command, op_filter):
    +    "get tests by --gtest_filter and --gtest_list_tests"
    +    list_command += ['--gtest_filter=' + op_filter]
    +    try:
    +      test_list = subprocess.Popen(list_command + ['--gtest_list_tests'],
    +                                 stdout=subprocess.PIPE).communicate()[0]
    +    except OSError as e:
    +      sys.exit("%s: %s" % (test_binary, str(e)))
    +
    +    command += additional_args
    +    tests = []
    +    test_group = ''
    +    for line in test_list.split('\n'):
    +      if not line.strip():
    --- End diff --
    
    If you use `test_list.split()`, it will handle this case. For example, `'a\n\nc\n'.split()` will return `['a', 'c']`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1007: HAWQ-1148. Update gtest-parallel to make ...

Posted by amyrazz44 <gi...@git.apache.org>.
Github user amyrazz44 closed the pull request at:

    https://github.com/apache/incubator-hawq/pull/1007


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1007: HAWQ-1148. Update gtest-parallel to make ...

Posted by amyrazz44 <gi...@git.apache.org>.
Github user amyrazz44 commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1007#discussion_r87322353
  
    --- Diff: src/test/feature/gtest-parallel ---
    @@ -234,6 +234,71 @@ class TestTimes(object):
         except IOError:
           pass  # ignore errors---saving the times isn't that important
     
    +
    +def get_schedule(schedule_file):
    +  "read schedule file from --gtest_schedule option"
    +  try:
    +    file = open(schedule_file, 'r')
    --- End diff --
    
    Will refine this, thank you.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1007: HAWQ-1148. Update gtest-parallel to make ...

Posted by paul-guo- <gi...@git.apache.org>.
Github user paul-guo- commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1007#discussion_r87321265
  
    --- Diff: src/test/feature/gtest-parallel ---
    @@ -260,6 +325,8 @@ parser.add_option('--gtest_color', type='string', default='yes',
                       help='color output')
     parser.add_option('--gtest_filter', type='string', default='',
                       help='test filter')
    +parser.add_option('--gtest_schedule', type='string', default='',
    +                  help='test schedule')
     parser.add_option('--gtest_also_run_disabled_tests', action='store_true',
                       default=False, help='run disabled tests too')
     parser.add_option('--format', type='string', default='filter',
    --- End diff --
    
    Well. The help info is just a copy of option string. That is not good I think. We should make users quick and easy to understand just from the help info.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1007: HAWQ-1148. Update gtest-parallel to make ...

Posted by xunzhang <gi...@git.apache.org>.
Github user xunzhang commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1007#discussion_r87318797
  
    --- Diff: src/test/feature/gtest-parallel ---
    @@ -285,41 +354,42 @@ else:
     save_file = os.path.join(os.path.expanduser("~"), ".gtest-parallel-times")
     times = TestTimes(save_file)
     tests = []
    +#pull all the tests into test_map dict, in order to mark the failed test in FAILED log 
    +test_map = {} 
    +# mark the end of paralell test id by parallel_id
    +parallel_id = 0
    +
     for test_binary in binaries:
       command = [test_binary]
       if options.gtest_also_run_disabled_tests:
         command += ['--gtest_also_run_disabled_tests']
    -
       list_command = list(command)
    +  
    +  if options.gtest_schedule != '' and options.gtest_filter != '':
    +    sys.exit("Option input failure : gtest_schedule and gtest_filter can not use in the same time: \n")
    +    
    +  if options.gtest_schedule != '':
    --- End diff --
    
    `if options.gtest_schedule `


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq issue #1007: HAWQ-1148. Update gtest-parallel to make sure te...

Posted by paul-guo- <gi...@git.apache.org>.
Github user paul-guo- commented on the issue:

    https://github.com/apache/incubator-hawq/pull/1007
  
    +1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1007: HAWQ-1148. Update gtest-parallel to make ...

Posted by xunzhang <gi...@git.apache.org>.
Github user xunzhang commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1007#discussion_r87318783
  
    --- Diff: src/test/feature/gtest-parallel ---
    @@ -285,41 +354,42 @@ else:
     save_file = os.path.join(os.path.expanduser("~"), ".gtest-parallel-times")
     times = TestTimes(save_file)
     tests = []
    +#pull all the tests into test_map dict, in order to mark the failed test in FAILED log 
    +test_map = {} 
    +# mark the end of paralell test id by parallel_id
    +parallel_id = 0
    +
     for test_binary in binaries:
       command = [test_binary]
       if options.gtest_also_run_disabled_tests:
         command += ['--gtest_also_run_disabled_tests']
    -
       list_command = list(command)
    +  
    +  if options.gtest_schedule != '' and options.gtest_filter != '':
    --- End diff --
    
    `if options.gtest_schedule  and options.gtest_filter:`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1007: HAWQ-1148. Update gtest-parallel to make ...

Posted by amyrazz44 <gi...@git.apache.org>.
Github user amyrazz44 commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1007#discussion_r87323443
  
    --- Diff: src/test/feature/gtest-parallel ---
    @@ -285,41 +354,42 @@ else:
     save_file = os.path.join(os.path.expanduser("~"), ".gtest-parallel-times")
     times = TestTimes(save_file)
     tests = []
    +#pull all the tests into test_map dict, in order to mark the failed test in FAILED log 
    +test_map = {} 
    +# mark the end of paralell test id by parallel_id
    +parallel_id = 0
    +
     for test_binary in binaries:
       command = [test_binary]
       if options.gtest_also_run_disabled_tests:
         command += ['--gtest_also_run_disabled_tests']
    -
       list_command = list(command)
    +  
    +  if options.gtest_schedule != '' and options.gtest_filter != '':
    +    sys.exit("Option input failure : gtest_schedule and gtest_filter can not use in the same time: \n")
    +    
    +  if options.gtest_schedule != '':
    +    (pcount, filter_test) = get_schedule(options.gtest_schedule)
    +    for i in range(0, len(filter_test)):
    --- End diff --
    
    Will consider your suggestion in the future, while in this script this way will be more suitable due to the deal with test_map. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request #1007: HAWQ-1148. Update gtest-parallel to make ...

Posted by xunzhang <gi...@git.apache.org>.
Github user xunzhang commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/1007#discussion_r87317889
  
    --- Diff: src/test/feature/gtest-parallel ---
    @@ -234,6 +234,71 @@ class TestTimes(object):
         except IOError:
           pass  # ignore errors---saving the times isn't that important
     
    +
    +def get_schedule(schedule_file):
    +  "read schedule file from --gtest_schedule option"
    +  try:
    +    file = open(schedule_file, 'r')
    --- End diff --
    
    Do not use default keyword `file`, it is risky.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---