Mailing-List: contact dev-help@falcon.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@falcon.apache.org
Content-Type: multipart/alternative;
 boundary="===============4213800557059950668=="
MIME-Version: 1.0
Subject: Re: Review Request 34240: Feed Wizard multiple tests
From: "Raghav Gautam" <raghav@apache.org>
To: "Namit Maheshwari" <nmaheshwari@hortonworks.com>,
 "Raghav Gautam" <raghav@apache.org>,
 "Falcon" <dev@falcon.incubator.apache.org>
Date: Thu, 21 May 2015 00:16:44 -0000
Message-ID: <20150521001644.17474.62057@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "Raghav Gautam" <noreply@reviews.apache.org>
References: <20150520014926.14202.79720@reviews.apache.org>
In-Reply-To: <20150520014926.14202.79720@reviews.apache.org>
Reply-To: "Raghav Gautam" <raghav@apache.org>

--===============4213800557059950668==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


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


falcon-regression/merlin/src/main/java/org/apache/falcon/regression/ui/search/AbstractSearchPage.java
<https://reviews.apache.org/r/34240/#comment135962>

    In general, it is better to use list instead of arrays. The benefits are better toString and being able to search for element without for loop.


falcon-regression/merlin/src/test/java/org/apache/falcon/regression/searchUI/FeedSetupTest.java
<https://reviews.apache.org/r/34240/#comment135966>

    Assert failure without appropriate message don't convey much. It will be great if you can add appropriate messages to be printed when assert fails.
    
    Applicable to other assertions as well.


falcon-regression/merlin/src/test/java/org/apache/falcon/regression/searchUI/FeedSetupTest.java
<https://reviews.apache.org/r/34240/#comment135969>

    You can set tags and groups even if they are not null ?
    
    In case you need to do the check, using StringUtil.isEmpty() would be more robust.
    
    Applicable to other places also.


falcon-regression/merlin/src/test/java/org/apache/falcon/regression/searchUI/FeedSetupTest.java
<https://reviews.apache.org/r/34240/#comment135973>

    Is this string same as the one that has been set a few lines above. In that case extract a constant so, that this fact is more obvious.
    
    If not, then how did you come up with this string ?


falcon-regression/merlin/src/test/java/org/apache/falcon/regression/searchUI/FeedSetupTest.java
<https://reviews.apache.org/r/34240/#comment135974>

    Can you use all the path to be starting with prefix baseTestHDFSDir ?
    
    That way by looking at the path in the printed log/screenshot etc, we would know what test are we talking about.
    
    Applicable at multiple places.


- Raghav Gautam


On May 19, 2015, 6:49 p.m., Namit Maheshwari wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34240/
> -----------------------------------------------------------
> 
> (Updated May 19, 2015, 6:49 p.m.)
> 
> 
> Review request for Falcon.
> 
> 
> Bugs: FALCON-1222
>     https://issues.apache.org/jira/browse/FALCON-1222
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> Feed Wizard multiple tests
> 
> 
> Diffs
> -----
> 
>   falcon-regression/merlin/src/main/java/org/apache/falcon/regression/ui/search/AbstractSearchPage.java e72ce6706d7fe3331744d723894193bfbaff586a 
>   falcon-regression/merlin/src/main/java/org/apache/falcon/regression/ui/search/FeedWizardPage.java 6eed4cef86c1974c3d30f0133b34894e24ac1750 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/searchUI/FeedSetupTest.java 8920f41129175a207ae6dee0659a514fef17bcfc 
> 
> Diff: https://reviews.apache.org/r/34240/diff/
> 
> 
> Testing
> -------
> 
> Below new tests added. Tested the same.
> testWizardCancel
> testWizardXmlPreview
> testGeneralStepEditXml
> testGeneralStepDefaultScenario
> testGeneralStepXmlPreview
> testGeneralStepAddRemoveTag
> testGeneralStepBlankOptionalFields
> testTimingStepDefaultScenario
> testTimingStepXmlPreview
> testTimingStepEditXml
> testTimingStepDropDownLists
> testTimingStepAddDeleteProperties
> testLocationStepValidValuesFS
> testLocationStepValidCatalogStorage
> testLocationStepBothLocationsAndTableUri
> testLocationStepEditXml
> testClustersStepDefaultScenario
> testClustersStepDropDownLists
> testClustersStepDefaultLocations
> testClusterStepEditXml
> testSummaryStepDefaultScenario
> 
> 
> Thanks,
> 
> Namit Maheshwari
> 
>


--===============4213800557059950668==--