You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@falcon.apache.org by Ajay Yadava <aj...@gmail.com> on 2016/01/04 11:48:30 UTC

Re: Review Request 41747: [FALCON-1698] New tests for ProcessSetupTest, ClusterSetupTest, UI test fixes

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



falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/Entities/ClusterMerlin.java (line 102)
<https://reviews.apache.org/r/41747/#comment173028>

    Will StringUtils.isNotBlank be better?



falcon-regression/merlin/src/main/java/org/apache/falcon/regression/ui/search/ClusterWizardPage.java (line 295)
<https://reviews.apache.org/r/41747/#comment173032>

    nit: typo, should be "Assertion for working directory"



falcon-regression/merlin/src/main/java/org/apache/falcon/regression/ui/search/EntityPage.java (line 409)
<https://reviews.apache.org/r/41747/#comment173033>

    Why is the timeout OS specific?



falcon-regression/merlin/src/main/java/org/apache/falcon/regression/ui/search/PageHeader.java (line 189)
<https://reviews.apache.org/r/41747/#comment173034>

    Just curious, what's the advantage of doing it this way instead of just doing link.click()? Is the intent to make it a common function for clicking any type of element rather than just link or is there more to it?


- Ajay Yadava


On Dec. 28, 2015, 6:30 p.m., Paul Isaychuk wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41747/
> -----------------------------------------------------------
> 
> (Updated Dec. 28, 2015, 6:30 p.m.)
> 
> 
> Review request for Falcon.
> 
> 
> Bugs: FALCON-1698
>     https://issues.apache.org/jira/browse/FALCON-1698
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> Adding new tests to ProcessSetupTest, ClusterSetupTest; UI test fixes related to UI changes.
> 
> 
> Diffs
> -----
> 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/Entities/ClusterMerlin.java fe1338a 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/Entities/ProcessMerlin.java 7607aa6 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/bundle/Bundle.java 67d9ee2 
>   falcon-regression/merlin/src/main/java/org/apache/falcon/regression/ui/search/ClusterWizardPage.java bcada4a 
>   falcon-regression/merlin/src/main/java/org/apache/falcon/regression/ui/search/EntityPage.java f7499b7 
>   falcon-regression/merlin/src/main/java/org/apache/falcon/regression/ui/search/PageHeader.java 7f87091 
>   falcon-regression/merlin/src/main/java/org/apache/falcon/regression/ui/search/ProcessWizardPage.java 5dcd700 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/searchUI/ClusterSetupTest.java 5efa5b2 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/searchUI/MirrorSummaryTest.java 989e4b3 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/searchUI/ProcessSetupTest.java eae5137 
> 
> Diff: https://reviews.apache.org/r/41747/diff/
> 
> 
> Testing
> -------
> 
> done
> 
> 
> Thanks,
> 
> Paul Isaychuk
> 
>


Re: Review Request 41747: [FALCON-1698] New tests for ProcessSetupTest, ClusterSetupTest, UI test fixes

Posted by Paul Isaychuk <pi...@hortonworks.com>.

> On Jan. 4, 2016, 10:48 a.m., Ajay Yadava wrote:
> > falcon-regression/merlin/src/main/java/org/apache/falcon/regression/ui/search/EntityPage.java, line 409
> > <https://reviews.apache.org/r/41747/diff/1/?file=1176922#file1176922line409>
> >
> >     Why is the timeout OS specific?

We faced a need for this only on windows.


> On Jan. 4, 2016, 10:48 a.m., Ajay Yadava wrote:
> > falcon-regression/merlin/src/main/java/org/apache/falcon/regression/ui/search/PageHeader.java, line 192
> > <https://reviews.apache.org/r/41747/diff/1/?file=1176923#file1176923line192>
> >
> >     Just curious, what's the advantage of doing it this way instead of just doing link.click()? Is the intent to make it a common function for clicking any type of element rather than just link or is there more to it?

This method created to click on links which selenium can't click by it's own (there are issues when selenium can't get actual element coordinates e.g. https://code.google.com/p/selenium/issues/detail?id=6756)


> On Jan. 4, 2016, 10:48 a.m., Ajay Yadava wrote:
> > falcon-regression/merlin/src/main/java/org/apache/falcon/regression/ui/search/ClusterWizardPage.java, line 296
> > <https://reviews.apache.org/r/41747/diff/1/?file=1176921#file1176921line296>
> >
> >     nit: typo, should be "Assertion for working directory"

Accidentely added a part of code from another patch. Removing it.


> On Jan. 4, 2016, 10:48 a.m., Ajay Yadava wrote:
> > falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/Entities/ClusterMerlin.java, line 102
> > <https://reviews.apache.org/r/41747/diff/1/?file=1176918#file1176918line102>
> >
> >     Will StringUtils.isNotBlank be better?

Sure, it's better. Thanks.


- Paul


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


On Dec. 28, 2015, 6:30 p.m., Paul Isaychuk wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41747/
> -----------------------------------------------------------
> 
> (Updated Dec. 28, 2015, 6:30 p.m.)
> 
> 
> Review request for Falcon.
> 
> 
> Bugs: FALCON-1698
>     https://issues.apache.org/jira/browse/FALCON-1698
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> Adding new tests to ProcessSetupTest, ClusterSetupTest; UI test fixes related to UI changes.
> 
> 
> Diffs
> -----
> 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/Entities/ClusterMerlin.java fe1338a 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/Entities/ProcessMerlin.java 7607aa6 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/bundle/Bundle.java 67d9ee2 
>   falcon-regression/merlin/src/main/java/org/apache/falcon/regression/ui/search/ClusterWizardPage.java bcada4a 
>   falcon-regression/merlin/src/main/java/org/apache/falcon/regression/ui/search/EntityPage.java f7499b7 
>   falcon-regression/merlin/src/main/java/org/apache/falcon/regression/ui/search/PageHeader.java 7f87091 
>   falcon-regression/merlin/src/main/java/org/apache/falcon/regression/ui/search/ProcessWizardPage.java 5dcd700 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/searchUI/ClusterSetupTest.java 5efa5b2 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/searchUI/MirrorSummaryTest.java 989e4b3 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/searchUI/ProcessSetupTest.java eae5137 
> 
> Diff: https://reviews.apache.org/r/41747/diff/
> 
> 
> Testing
> -------
> 
> done
> 
> 
> Thanks,
> 
> Paul Isaychuk
> 
>


Re: Review Request 41747: [FALCON-1698] New tests for ProcessSetupTest, ClusterSetupTest, UI test fixes

Posted by Ajay Yadava <aj...@gmail.com>.

> On Jan. 4, 2016, 10:48 a.m., Ajay Yadava wrote:
> > falcon-regression/merlin/src/main/java/org/apache/falcon/regression/ui/search/EntityPage.java, line 409
> > <https://reviews.apache.org/r/41747/diff/1/?file=1176922#file1176922line409>
> >
> >     Why is the timeout OS specific?
> 
> Paul Isaychuk wrote:
>     We faced a need for this only on windows.

I always knew Linux is faster :P 

On a serious note, I feel this kind of issue may be machine dependent and might occur on other low configuration hardware even in Linux(we have seen some falcon unit tests fail on mac laptops while they run pretty smoothly on my high end ubuntu desktop), so it will be better to keep a sleep irrespective of OS if it doesn't degrade the performance of tests singificantly.


- Ajay


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


On Jan. 4, 2016, 12:53 p.m., Paul Isaychuk wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41747/
> -----------------------------------------------------------
> 
> (Updated Jan. 4, 2016, 12:53 p.m.)
> 
> 
> Review request for Falcon.
> 
> 
> Bugs: FALCON-1698
>     https://issues.apache.org/jira/browse/FALCON-1698
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> Adding new tests to ProcessSetupTest, ClusterSetupTest; UI test fixes related to UI changes.
> 
> 
> Diffs
> -----
> 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/Entities/ClusterMerlin.java fe1338a 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/Entities/ProcessMerlin.java 7607aa6 
>   falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/bundle/Bundle.java 67d9ee2 
>   falcon-regression/merlin/src/main/java/org/apache/falcon/regression/ui/search/ClusterWizardPage.java bcada4a 
>   falcon-regression/merlin/src/main/java/org/apache/falcon/regression/ui/search/EntityPage.java f7499b7 
>   falcon-regression/merlin/src/main/java/org/apache/falcon/regression/ui/search/PageHeader.java 7f87091 
>   falcon-regression/merlin/src/main/java/org/apache/falcon/regression/ui/search/ProcessWizardPage.java 5dcd700 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/searchUI/ClusterSetupTest.java 5efa5b2 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/searchUI/MirrorSummaryTest.java 989e4b3 
>   falcon-regression/merlin/src/test/java/org/apache/falcon/regression/searchUI/ProcessSetupTest.java eae5137 
> 
> Diff: https://reviews.apache.org/r/41747/diff/
> 
> 
> Testing
> -------
> 
> done
> 
> 
> Thanks,
> 
> Paul Isaychuk
> 
>