Skip to content

[ZEPPELIN-2467] Restoring second travis job functionality #2300

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
AlexanderShoshin wants to merge 3 commits into from
Closed

[ZEPPELIN-2467] Restoring second travis job functionality #2300

AlexanderShoshin wants to merge 3 commits into from

Conversation

@AlexanderShoshin
Copy link
Contributor

@AlexanderShoshin AlexanderShoshin commented Apr 28, 2017 •
edited
Loading

What is this PR for?

Second travis job didn't work correctly. It suppose to test all core unit tests plus integration test (except for spark related test) but it was doing nothing.

It was because of the exclamation mark in -Dtest property:

-Dtest='!ZeppelinSparkClusterTest,!org.apache.zeppelin.spark.*'

which is not supported by maven-surefire-plugin of version 2.17 (which is used in Zeppelin). Exclamation mark is supported started from 2.19 but still does not work properly.

I've added plugin exclude configuratin instead of -Dtest property.

After travis job was restored I had found that not all core tests were working properly. I have excluded them from the travis job for now and created jira issues:
https://issues.apache.org/jira/browse/ZEPPELIN-2469
https://issues.apache.org/jira/browse/ZEPPELIN-2470
https://issues.apache.org/jira/browse/ZEPPELIN-2471
https://issues.apache.org/jira/browse/ZEPPELIN-2473

What type of PR is it?

[Bug Fix]

What is the Jira issue?

ZEPPELIN-2467

Questions:

  • Does the licenses files need update? no
  • Is there breaking changes for older versions? no
  • Does this needs documentation? no

felixcheung
felixcheung reviewed Apr 30, 2017
View reviewed changes
.travis.yml
# Test core modules
- jdk: "oraclejdk7"
env: SCALA_VER="2.11" SPARK_VER="2.1.0" HADOOP_VER="2.6" PROFILE="-Pweb-ci -Pscalding -Phelium-dev -Pexamples -Pscala-2.11" BUILD_FLAG="package -Pbuild-distr -DskipRat" TEST_FLAG="verify -Pusing-packaged-distr -DskipRat" MODULES="-pl ${INTERPRETERS}" TEST_PROJECTS="-Dtest='!ZeppelinSparkClusterTest,!org.apache.zeppelin.spark.*' -DfailIfNoTests=false"
env: SCALA_VER="2.11" SPARK_VER="2.1.0" HADOOP_VER="2.6" PROFILE="-Pweb-ci -Pscalding -Phelium-dev -Pexamples -Pscala-2.11" BUILD_FLAG="package -Pbuild-distr -DskipRat" TEST_FLAG="verify -Pusing-packaged-distr -DskipRat" MODULES="-pl ${INTERPRETERS}" TEST_PROJECTS="-Dtests.to.exclude=**/ZeppelinSparkClusterTest.java,**/org.apache.zeppelin.spark.*,**/HeliumBundleFactoryTest.java,**/HeliumApplicationFactoryTest.java,**/NotebookTest.java,**/ZeppelinRestApiTest.java -DfailIfNoTests=false"
Copy link
Member

@felixcheung felixcheung Apr 30, 2017

Choose a reason for hiding this comment

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

excluding the exact filename is a bit hard to maintain, I think? is there a way to exclude by subfolder/path?

Copy link
Contributor Author

@AlexanderShoshin AlexanderShoshin May 2, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I agree that excluding tests by direct links is not the best way to solve the problem. But I couldn't think up anything more appropriate.

Excluding tests by packages will cause 15 more test files won't run because our 4 tests are located in 3 different packages in 2 different modules.

Maybe the better way will be to add an @Ignore annotation into test files instead of excluding them using travis configuration?

Copy link
Member

@Leemoonsoo Leemoonsoo May 2, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Or how about add some verbose comments a line above to explain each file in tests.to.exclude flag?
The comments can mention JIRA issue. Then i think everyone can easily understand reason why the file is excluded and helps maintaining the travis.yml

Copy link
Contributor Author

@AlexanderShoshin AlexanderShoshin May 3, 2017

Choose a reason for hiding this comment

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

I've added a comment about excluded tests

@AlexanderShoshin
Copy link
Contributor Author

AlexanderShoshin commented May 3, 2017

According to @Leemoonsoo suggestion I have added a comment for the travis job configuration about excluded tests.

@Leemoonsoo
Copy link
Member

Leemoonsoo commented May 5, 2017

Travis build green. https://travis-ci.org/AlexanderShoshin/zeppelin/builds/228261473

LGTM and merge to master if no further discussions.

@asfgit asfgit closed this in 15ecbb4 May 6, 2017
This was referenced May 10, 2017
Closed
Closed
Closed
Closed
@1ambda
Copy link
Member

1ambda commented May 10, 2017 •
edited
Loading

even though CI was green in this PR at that time, it seems that wasn't rebased recently. As a result CI is failing only in the second travis JOB. @AlexanderShoshin Could you check that?

michelemilesi pushed a commit to icteam-spa/zeppelin that referenced this pull request May 11, 2017
@AlexanderShoshin @michelemilesi
[ZEPPELIN-2467] Restoring second travis job functionality
6890726 
### What is this PR for?
Second travis job didn't work correctly. It suppose to test all core unit tests plus integration test (except for spark related test) but it was doing nothing.

It was because of the exclamation mark in **-Dtest** property:
```
-Dtest='!ZeppelinSparkClusterTest,!org.apache.zeppelin.spark.*'
```
which is not supported by maven-surefire-plugin of version 2.17 (which is used in Zeppelin). Exclamation mark is supported started from 2.19 but still does not work properly.

I've added plugin **exclude** configuratin instead of **-Dtest** property.

After travis job was restored I had found that not all core tests were working properly. I have excluded them from the travis job for now and created jira issues:
https://issues.apache.org/jira/browse/ZEPPELIN-2469
https://issues.apache.org/jira/browse/ZEPPELIN-2470
https://issues.apache.org/jira/browse/ZEPPELIN-2471
https://issues.apache.org/jira/browse/ZEPPELIN-2473

### What type of PR is it?
[Bug Fix]

### What is the Jira issue?
[ZEPPELIN-2467](https://issues.apache.org/jira/browse/ZEPPELIN-2467)

### Questions:
* Does the licenses files need update? **no**
* Is there breaking changes for older versions? **no**
* Does this needs documentation? **no**

Author: Alexander Shoshin <[email protected]>

Closes apache#2300 from AlexanderShoshin/ZEPPELIN-2467 and squashes the following commits:

78771e9 [Alexander Shoshin] made a comment about excluded tests
dfa332f [Alexander Shoshin] changed -Dtest flag to -Dtests.to.exclude and excluded unstable tests
0448c4a [Alexander Shoshin] added ability to exclude some unit tests using command line
@jongyoul
Copy link
Member

jongyoul commented May 12, 2017

@AlexanderShoshin After merging this PR, we continued to got failed in CI. Can you handle it urgently? If it took some times, it would be better to revert this commit and merge it after fixing it. How do you think of it?

@AlexanderShoshin
Copy link
Contributor Author

AlexanderShoshin commented May 12, 2017

I think we should revert this commit then. I won't be able to fix it fast.

@jongyoul
Copy link
Member

jongyoul commented May 12, 2017

Thanks for your opinion. Then I'll revert it. Please reopen this PR and rebase it from current master.

@Leemoonsoo Leemoonsoo mentioned this pull request May 14, 2017
Closed
asfgit pushed a commit that referenced this pull request May 17, 2017
@AlexanderShoshin @Leemoonsoo
[ZEPPELIN-2467] Restore travis second job functionality
9cf09cc 
### What is this PR for?
issue ZEPPELIN-2467 has resolved by #2300. However 8194a5e reverts #2300.

This PR apply #2300 again with two more test exclusion:
  - SecurityRestApiTest (https://issues.apache.org/jira/browse/ZEPPELIN-2545)
  - NotebookSecurityRestApiTest (https://issues.apache.org/jira/browse/ZEPPELIN-2546)

### What type of PR is it?
Bug Fix

### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-2467

### How should this be tested?
CI becomes green

### Questions:
* Does the licenses files need update? no
* Is there breaking changes for older versions? no
* Does this needs documentation? no

Author: Alexander Shoshin <[email protected]>
Author: Lee moon soo <[email protected]>

Closes #2342 from Leemoonsoo/fix_ci2 and squashes the following commits:

1a8f24b [Lee moon soo] exclude SecurityRestApiTest and NotebookSecurityRestApiTest
4f82243 [Alexander Shoshin] made a comment about excluded tests
95eb7be [Alexander Shoshin] changed -Dtest flag to -Dtests.to.exclude and excluded unstable tests
41af70f [Alexander Shoshin] added ability to exclude some unit tests using command line
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@felixcheung felixcheung felixcheung left review comments

+1 more reviewer

@Leemoonsoo Leemoonsoo Leemoonsoo left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@AlexanderShoshin @Leemoonsoo @1ambda @jongyoul @felixcheung