brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From neykov <...@git.apache.org>
Subject [GitHub] incubator-brooklyn pull request: Fix UsageResourceTest.testListApp...
Date Wed, 18 Mar 2015 17:01:49 GMT
Github user neykov commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/558#issuecomment-83057921
  
    I was the last one who took a stab at fixing this test so can provide some context.
    The 1ms delay is on purpose due to the check in `UsageResource`
    ```
                if (eventStartDate.compareTo(endDate) > 0 || eventEndDate.compareTo(startDate)
< 0) {
                    continue;
                }
    ```
    We have to make sure that we are requesting times at least 1ms *after* `eventStartDate`
(thus the +1ms). The `waitForFuture(postStart+1ms)` is because `eventEndDate=new Date()`,
to make sure that the requested time is not in the future. The comparison is done on the full
values, they are cropped at serialization time only, so no need to add 1s.
    
    Your other half of the fix is correct though, the test should assert against `afterPostStart`
due to
    ```
                if (eventStartDate.compareTo(startDate) < 0) {
                    eventStartDate = startDate;
                }
                if (eventEndDate.compareTo(endDate) > 0) {
                    eventEndDate = endDate;
                }
    ```
    
    I can see another potential problem. It could (quite unlikely) happen that `afterPostStart`
is equal to `eventEndDate = new Date()` Due to the `eventEndDate.compareTo(startDate) <
0` the test will fail. To make sure this doesn't happen I'd suggest changing the original
test to
    ```
    long afterPostStart = postStart.getTime()+1;
    waitForFuture(afterPostStart+1);
    ```
    so that we are completely sure that `afterPostStart` is at least 1ms after the entity
start time and at least 1ms before `new Date()`.


---
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.
---

Mime
View raw message