Skip to content

ZEPPELIN-593 Test Paragraph action Disable run and refactor common methods #619

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
r-kamath wants to merge 4 commits into from
Closed

ZEPPELIN-593 Test Paragraph action Disable run and refactor common methods #619

r-kamath wants to merge 4 commits into from

Conversation

@r-kamath
Copy link
Member

@r-kamath r-kamath commented Jan 11, 2016

What is this PR for?

Add new selenium test for disable paragraph action and refactor some of the common methods which can be used by other tests.

What type of PR is it?

Improvement and refactor

Is there a relevant Jira issue?

ZEPPELIN-593

How should this be tested?

CI should pass

or

On OSX, you'll need firefox 42.0 installed, then you can run with
PATH=~/Applications/Firefox.app/Contents/MacOS/:$PATH CI="" mvn -Dtest=org.apache.zeppelin.integration.ParagraphActions -Denforcer.skip=true test -pl zeppelin-server

@prabhjyotsingh
Copy link
Contributor

prabhjyotsingh commented Jan 11, 2016

awesome, more selenium 👍

@bzz
Copy link
Member

bzz commented Jan 11, 2016

More integration tests is very welcome, so thank you!
Having test utils separated from the main body of test itself sounds like a good idea.

What do you think if instead of having class TestUtils + TestUtils.xxx(..., driver) calls everywhere on the client, we move all those goodies to something like abstract class AbstractZeppelinIT and have class ZeppelinIT extends AbstractZeppelinIT ?

This way we can still have all that code separated and available as part of skeletal implementation for every other test, each will possess a driver instance and the client code will look simpler and shorter: something(...) vs TestUtils.something(..., driver)

@r-kamath
Copy link
Member Author

r-kamath commented Jan 13, 2016

@bzz I've updated the patch as per the review comment. Thanks

@r-kamath
Copy link
Member Author

r-kamath commented Jan 18, 2016

@bzz @Leemoonsoo ready for review

@prabhjyotsingh
Copy link
Contributor

prabhjyotsingh commented Jan 18, 2016

LGTM. Tested on MacOS with

PATH=~/Applications/Firefox.app/Contents/MacOS/:$PATH CI="" \
mvn -Dtest=org.apache.zeppelin.integration.ParagraphActions -Denforcer.skip=true \
test -pl zeppelin-server

runs as expected.

@prabhjyotsingh prabhjyotsingh mentioned this pull request Jan 18, 2016
Closed
4 tasks
@prabhjyotsingh
Copy link
Contributor

prabhjyotsingh commented Jan 21, 2016

Can we have this reviewed/merged, #654 can use some refactored stuff.

bzz
bzz reviewed Jan 21, 2016
View reviewed changes
zeppelin-server/src/test/java/org/apache/zeppelin/AbstractZeppelinIT.java Outdated
Copy link
Member

@bzz bzz Jan 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like a small typo here

@bzz
Copy link
Member

bzz commented Jan 21, 2016

Great job @r-kamath thank you for contributing!

Looks good to me, modulo minor nits above.

@bzz
Copy link
Member

bzz commented Jan 22, 2016

Will merge if there is no more discussion

@asfgit asfgit closed this in 61bc279 Jan 22, 2016
prabhjyotsingh added a commit to prabhjyotsingh/zeppelin that referenced this pull request Feb 3, 2016
@prabhjyotsingh
missed refactor with apache#619
2c28758
asfgit pushed a commit that referenced this pull request Feb 21, 2016
@prabhjyotsingh @Leemoonsoo
selenium test spark, pyspark and sparkSql
9b72c89 
### What is this PR for?
Test functionality of spark, pyspark, sparksql

### What type of PR is it?
Improvement

### Todos
* [x] - Selenium for spark
* [x] - Selenium for pyspark
* [x] - Selenium for sparksql
* [x] - refactor with #619

### Is there a relevant Jira issue?
ZEPPELIN-587

### How should this be tested?
On macOS

    PATH=~/Applications/Firefox.app/Contents/MacOS/:$PATH CI="" \
    mvn -Dtest=org.apache.zeppelin.integration.TestSparkParagraph -Denforcer.skip=true \
    test -pl zeppelin-server

Author: Prabhjyot Singh <prabhjyotsingh@gmail.com>

Closes #654 from prabhjyotsingh/ZEPPELIN-587 and squashes the following commits:

8f24695 [Prabhjyot Singh] use handleException in all other test cases remove test for spark 1.1.1 more meaningful log message
28dfc55 [Prabhjyot Singh] thorwong exception similar to #709
21bcc45 [Prabhjyot Singh] Merge remote-tracking branch 'origin/master' into ZEPPELIN-587
b05b81b [Prabhjyot Singh] have SHIFT_ENTER enum
9a206f4 [Prabhjyot Singh] add missing endToEndTestEnabled check for testSparkInterpreterDependencyLoading
ab03287 [Prabhjyot Singh] have static import of AbstractZeppelinIT.HelperKeys.*, and rg.openqa.selenium.Keys.*
5187a16 [Prabhjyot Singh] check if spark version is less than 1.3 then don't append ".toDF()"
6927f7e [Prabhjyot Singh] CI FIX
9236e3c [Prabhjyot Singh] implemeting @bzz review comments
2c28758 [Prabhjyot Singh] missed refactor with #619
d49344f [Prabhjyot Singh] selenium test spark, pyspark and sparkSql
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@r-kamath @prabhjyotsingh @bzz