falcon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Raghav Gautam" <rag...@apache.org>
Subject Re: Review Request 33396: [FALCON-1171] Adding tests for search API
Date Thu, 23 Apr 2015 00:10:50 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33396/#review81231
-----------------------------------------------------------



falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SearchApiTest.java
<https://reviews.apache.org/r/33396/#comment131527>

    The way I understoop this scenario was by constructing this matrix. You may want to add
this as part of documentation in this class.
    
    |--------------+-----------+------------+-----------------------------+-----------------------------+-----------------------------|
    | Feed Name    | inputFeed | outputFeed | bundle1-feed                | bundle2-feed 
              | bundle3-feed                |
    | Tags         |           |            | specific=bundle1            | specific=bundle2
           | specific=bundle3            |
    |              |           |            | common=common               | common=common
              | common=common               |
    |              |           |            | partial=b1b2                | partial=b1b2 
              |                             |
    |--------------+-----------+------------+-----------------------------+-----------------------------+-----------------------------|
    | Process Name |           |            | bundle1-process             | bundle2-process
            | bundle3-process             |
    | Tags         |           |            | specific=bundle1            | specific=bundle2
           | specific=bundle3            |
    |              |           |            | partial=b1b2                | partial=b1b2 
              |                             |
    |--------------+-----------+------------+-----------------------------+-----------------------------+-----------------------------|
    | Mirror Name  |           |            | bundle1-mirror-process      | bundle2-mirror-process
     | bundle3-mirror-process      |
    | Tags         |           |            | specific=bundle1            | specific=bundle2
           | specific=bundle3            |
    |              |           |            | _falcon_mirroring_type=HDFS | _falcon_mirroring_type=HIVE
| _falcon_mirroring_type=HDFS |
    |--------------+-----------+------------+-----------------------------+-----------------------------+-----------------------------|



falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SearchApiTest.java
<https://reviews.apache.org/r/33396/#comment131487>

    This method can be inlined. You can use params.getType() instead of varibale type. Same
thing for other variables.



falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SearchApiTest.java
<https://reviews.apache.org/r/33396/#comment131488>

    Is there a test for scenario:
    Search entities using 'schedulable'. 
    Check that all processes and feeds are present in response.
    I think in that case we expect 2 + 3 + 3 +3 = 11 results. I don't see that anywhere.



falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SearchApiTest.java
<https://reviews.apache.org/r/33396/#comment131491>

    Schedulable is a vaild type that is missing from the error message. You might want to
open a product bug also.



falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SearchApiTest.java
<https://reviews.apache.org/r/33396/#comment131498>

    assertNull ?



falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SearchApiTest.java
<https://reviews.apache.org/r/33396/#comment131515>

    Probably you want to convert elements to list.



falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SearchApiTest.java
<https://reviews.apache.org/r/33396/#comment131503>

    you probably want to use List for second parameter - try Arrays.asList() for creating
the list.



falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SearchApiTest.java
<https://reviews.apache.org/r/33396/#comment131516>

    Message in case of assertion failure ?



falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SearchApiTest.java
<https://reviews.apache.org/r/33396/#comment131494>

    Why are we removing inputFeed and outputFeed ? We expect entitie that have no tags to
also show up as part of results.



falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SearchApiTest.java
<https://reviews.apache.org/r/33396/#comment131517>

    Consider putting your assertions in this class.


- Raghav Gautam


On April 22, 2015, 9:20 a.m., Paul Isaychuk wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33396/
> -----------------------------------------------------------
> 
> (Updated April 22, 2015, 9:20 a.m.)
> 
> 
> Review request for Falcon.
> 
> 
> Bugs: FALCON-1171
>     https://issues.apache.org/jira/browse/FALCON-1171
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> Adding tests for search API
> 
> 
> Diffs
> -----
> 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/helpers/entity/AbstractEntityHelper.java
b899f1a 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/SearchApiTest.java
PRE-CREATION 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/entity/ListEntitiesTest.java
3ae44e6 
> 
> Diff: https://reviews.apache.org/r/33396/diff/
> 
> 
> Testing
> -------
> 
> All tests pass
> 
> 
> Thanks,
> 
> Paul Isaychuk
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message