Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.6.0
    • Fix Version/s: 0.7.0
    • Component/s: Tests
    • Labels:
      None

      Description

      The pig.jar path is hardcoded in the existing test-execution/smokes/pig/pom.xml file, which means that tarball based pig deployments or non-standard pig install locations cannot use the smoke test.

      Also, the pom.xml in test-execution/smokes could use a couple of small comments since it is different than the other test-executions as of this date (i.e. it depends on an external smoke rather than one created in test-artifacts).

      1. BIGTOP-1059.2.patch
        2 kB
        jay vyas
      2. BIGTOP-1059.1.patch
        2 kB
        jay vyas

        Activity

        Hide
        Mark Grover added a comment -

        As far as naming of the patches goes, I name them numbered just the way Jay is doing it, I prefer it that way because it namespaces the patches correctly on my personal workstation. In any case, Jay, it boils down to personal preference, so pick whichever way you like

        Show
        Mark Grover added a comment - As far as naming of the patches goes, I name them numbered just the way Jay is doing it, I prefer it that way because it namespaces the patches correctly on my personal workstation. In any case, Jay, it boils down to personal preference, so pick whichever way you like
        Hide
        Mark Grover added a comment -

        +1 and committed!

        FWIW, it looks you modified the original patch to create the new patch. That doesn't really work because of the way git creates patches (with hashes embedded in them for the good reason of reliability). The best way to avoid such problems is to make a commit in your own git repo and then regenerate the patch

        In any case, I modified the patch and committed. Thanks again for doing this!

        Show
        Mark Grover added a comment - +1 and committed! FWIW, it looks you modified the original patch to create the new patch. That doesn't really work because of the way git creates patches (with hashes embedded in them for the good reason of reliability). The best way to avoid such problems is to make a commit in your own git repo and then regenerate the patch In any case, I modified the patch and committed. Thanks again for doing this!
        Hide
        Konstantin Boudnik added a comment -

        No, no need, just saying for the future - it's a helpful way of tracking the progress/comments, etc.

        Show
        Konstantin Boudnik added a comment - No, no need, just saying for the future - it's a helpful way of tracking the progress/comments, etc.
        Hide
        jay vyas added a comment -

        ah okay . im okay to delete the old one

        Show
        jay vyas added a comment - ah okay . im okay to delete the old one
        Hide
        Konstantin Boudnik added a comment -

        BTW, you don't need to create custom suffixes for patch files - just keep uploading it under the same name, because JIRA will do the right thing for you.

        Show
        Konstantin Boudnik added a comment - BTW, you don't need to create custom suffixes for patch files - just keep uploading it under the same name, because JIRA will do the right thing for you.
        Hide
        jay vyas added a comment -

        Oh, and here is the updated patch with deleted version comment. .... thanks again for the reveiw mark/konstantin !

        Show
        jay vyas added a comment - Oh, and here is the updated patch with deleted version comment. .... thanks again for the reveiw mark/konstantin !
        Hide
        jay vyas added a comment -

        Are you sure this is the case for the pig tests? In any case , sure I'll remove the comment .. but meanwhile, (i think) the only pig tests in bigtop are for the pigsmokes, of which i think there is only one version published on maven central. (if im wrong please correct me, would love to run the official bigtop smokes for pig if there are some for pig 0.9.x or .10.x)...

        jays-MacBook-Pro:bigtop-apache Jpeerindex$ git checkout remotes/origin/branch-0.1
        Previous HEAD position was c4875b2... BIGTOP-12. Add HCatalog to Bigtop
        HEAD is now at cfb44ef... Excluding .gitignore files from dist for now.
        jays-MacBook-Pro:bigtop-apache Jpeerindex$ find ./ -name pig
        .//src/pkg/common/pig
        .//src/pkg/deb/pig
        .//src/pkg/rpm/pig
        .//test/suites/smokes/pig
        jays-MacBook-Pro:bigtop-apache Jpeerindex$ vim .//test/suites/smokes/pig/pom.xml
        jays-MacBook-Pro:bigtop-apache Jpeerindex$ cat .//test/suites/smokes/pig/pom.xml | grep smokes
        <artifactId>pigsmokes.smoke-tests</artifactId>
        <name>pigsmokes</name>

        Show
        jay vyas added a comment - Are you sure this is the case for the pig tests? In any case , sure I'll remove the comment .. but meanwhile, (i think) the only pig tests in bigtop are for the pigsmokes, of which i think there is only one version published on maven central. (if im wrong please correct me, would love to run the official bigtop smokes for pig if there are some for pig 0.9.x or .10.x)... jays-MacBook-Pro:bigtop-apache Jpeerindex$ git checkout remotes/origin/branch-0.1 Previous HEAD position was c4875b2... BIGTOP-12 . Add HCatalog to Bigtop HEAD is now at cfb44ef... Excluding .gitignore files from dist for now. jays-MacBook-Pro:bigtop-apache Jpeerindex$ find ./ -name pig .//src/pkg/common/pig .//src/pkg/deb/pig .//src/pkg/rpm/pig .//test/suites/smokes/pig jays-MacBook-Pro:bigtop-apache Jpeerindex$ vim .//test/suites/smokes/pig/pom.xml jays-MacBook-Pro:bigtop-apache Jpeerindex$ cat .//test/suites/smokes/pig/pom.xml | grep smokes <artifactId>pigsmokes.smoke-tests</artifactId> <name>pigsmokes</name>
        Hide
        Konstantin Boudnik added a comment -

        +1 on Mark's pick: please remove that comment.

        Show
        Konstantin Boudnik added a comment - +1 on Mark's pick: please remove that comment.
        Hide
        Mark Grover added a comment -

        I am +1 (with one minor nitpick below) and will wait for a day or so to commit in case someone else has any concerns with it.

        I would suggest, if you don't mind, to remove the comment regarding pig smoke test not working with pig < 0.11. Technically, the tests only have been run against the version of the component bundled in that version of Bigtop. However, sometimes we have to make changes in test code to make them compatible with the newer corresponding component version which may prevent them from running with the old version. We try to avoid doing so as much as we can but if someone really wanted to use tests with an older version of the component, they could get the test code from an older branch of Bigtop and use that instead.

        And, like always, thanks for contributing, Jay!

        Show
        Mark Grover added a comment - I am +1 (with one minor nitpick below) and will wait for a day or so to commit in case someone else has any concerns with it. I would suggest, if you don't mind, to remove the comment regarding pig smoke test not working with pig < 0.11. Technically, the tests only have been run against the version of the component bundled in that version of Bigtop. However, sometimes we have to make changes in test code to make them compatible with the newer corresponding component version which may prevent them from running with the old version. We try to avoid doing so as much as we can but if someone really wanted to use tests with an older version of the component, they could get the test code from an older branch of Bigtop and use that instead. And, like always, thanks for contributing, Jay!
        Hide
        jay vyas added a comment -

        Ive attached a patch for this above and tested it. The pig.jar property is properly picked up by the test-execution goal, and can indeed be something other than /usr/lib//pig/pig.jar now.

        Show
        jay vyas added a comment - Ive attached a patch for this above and tested it. The pig.jar property is properly picked up by the test-execution goal, and can indeed be something other than /usr/lib//pig/pig.jar now.
        Hide
        jay vyas added a comment -

        Patch with variable pig jar path option and a couple of comments on the way this test execution works.

        Show
        jay vyas added a comment - Patch with variable pig jar path option and a couple of comments on the way this test execution works.

          People

          • Assignee:
            jay vyas
            Reporter:
            jay vyas
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development