ZooKeeper
  1. ZooKeeper
  2. ZOOKEEPER-524

DBSizeTest is not really testing anything

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.3.0
    • Component/s: server, tests
    • Labels:
      None

      Description

      DBSizeTest looks like it should be testing latency, but it doesn't seem to do it (assert is commented out).

      We need to decide if this test should be fixed, or just dropped.

      Also note: this test takes 40seconds on my system. Way too long. Perhaps async create operations should be used
      to populate the database. I also noticed that data size has a big impact on overall test time (1k vs 5 bytes is something
      like a 2x time diff for time to run the test).

        Activity

        Hide
        Benjamin Reed added a comment -

        is there any objection to removing the test. (especially since it isn't doing anything?)

        Show
        Benjamin Reed added a comment - is there any objection to removing the test. (especially since it isn't doing anything?)
        Hide
        Benjamin Reed added a comment -

        there is no patch associated with this since i'm removing a file. i just wanted to make sure people saw this issue.

        Show
        Benjamin Reed added a comment - there is no patch associated with this since i'm removing a file. i just wanted to make sure people saw this issue.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org
        against trunk revision 903483.

        +1 @author. The patch does not contain any @author tags.

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no tests are needed for this patch.

        -1 patch. The patch command could not apply the patch.

        Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/63/console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org against trunk revision 903483. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no tests are needed for this patch. -1 patch. The patch command could not apply the patch. Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/63/console This message is automatically generated.
        Hide
        Flavio Junqueira added a comment -

        I must agree that the test awkward. Now, it takes 2 seconds on my laptop, so I don't understand why it takes 40 seconds on your computer, Pat.

        I think the idea of the test is not bad, but the implementation could be improved. For example, we could check if the latency if at most 10% higher after populating (10% is an arbitrary number, since it will be impossible to get exact values).

        If you guys feel strongly about removing it, I don't object and +1 it.

        Show
        Flavio Junqueira added a comment - I must agree that the test awkward. Now, it takes 2 seconds on my laptop, so I don't understand why it takes 40 seconds on your computer, Pat. I think the idea of the test is not bad, but the implementation could be improved. For example, we could check if the latency if at most 10% higher after populating (10% is an arbitrary number, since it will be impossible to get exact values). If you guys feel strongly about removing it, I don't object and +1 it.
        Hide
        Mahadev konar added a comment -

        its a little tricky to have latency tests as a part of unit testing. I think we should remove it. ben are you going to remove the file and commit?

        Show
        Mahadev konar added a comment - its a little tricky to have latency tests as a part of unit testing. I think we should remove it. ben are you going to remove the file and commit?
        Hide
        Benjamin Reed added a comment -

        Committed revision 912052.

        Show
        Benjamin Reed added a comment - Committed revision 912052.
        Hide
        Hudson added a comment -

        Integrated in ZooKeeper-trunk #702 (See http://hudson.zones.apache.org/hudson/job/ZooKeeper-trunk/702/)
        . DBSizeTest is not really testing anything

        Show
        Hudson added a comment - Integrated in ZooKeeper-trunk #702 (See http://hudson.zones.apache.org/hudson/job/ZooKeeper-trunk/702/ ) . DBSizeTest is not really testing anything

          People

          • Assignee:
            Benjamin Reed
            Reporter:
            Patrick Hunt
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development