Skip to content
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.

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

Disable NioTest #1538

Merged
merged 1 commit into from
May 12, 2016
Merged

Disable NioTest #1538

merged 1 commit into from
May 12, 2016

Conversation

rohityadavcloud
Copy link
Member

@rohityadavcloud rohityadavcloud commented May 11, 2016

Historically NioTest has caused issue in CI environment and several developer machines due to network requirements which could be disabled by firewall or security enforcers such as selinux. This disables the test once again using a historic commit 881a6e1
Signed-off-by: Rohit Yadav <rohit.yada

To build and just run this test: mvn clean install -pl utils -Dtest=NioTest

@rohityadavcloud
Copy link
Member Author

@swill @nvazquez @mike-tutkowski appreciate if we can discuss the issue here instead of ML, we'll lose discussion otherwise and I'm unable to keep up discussions at two places. Thanks.

Disable the NioTest, it is far to dependent on the network configuration of the
machine running the test. Cherry-picked change from a historic commit where
this test was disabled.

(cherry picked from commit 881a6e1)
Signed-off-by: Rohit Yadav <[email protected]>
@rohityadavcloud rohityadavcloud changed the title NioTest: relax test counts to pass under timeout Disable NioTest May 12, 2016
@mike-tutkowski
Copy link
Member

I should be able to try your branch out later today. Thanks!

On May 12, 2016, at 12:18 AM, Rohit Yadav <[email protected]mailto:[email protected]> wrote:

@swillhttps://github.com/swill @nvazquezhttps://github.com/nvazquez @mike-tutkowskihttps://github.com/mike-tutkowski appreciate if we can discuss the issue here instead of ML, we'll lose discussion otherwise and I'm unable to keep up discussions at two places. Thanks.

You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHubhttps://github.com//pull/1538#issuecomment-218669737

@DaanHoogland
Copy link
Contributor

LGTM

@mike-tutkowski
Copy link
Member

@rhtyd I was able to successfully run mvn -P developer,systemvm clean install -D noredist now on Mac OS X El Capitan 10.11.4, so LGTM.

@rafaelweingartner
Copy link
Member

Hi guys,
Please do not take me in a bad way, but why do we keep a test code that does not work? To me it is clearly that the test and the code being tested need a complete refactoring to enable true unit and integration tests (not functional ones). I was looking at the history of “utils\pom.xml” and it feels like those tests were being ignored since ever.

@swill
Copy link
Contributor

swill commented May 12, 2016

@rhtyd Can you re-push now that we have fixed the issue that is failing in travis? Thx...

@asfgit asfgit merged commit d4cb05b into apache:master May 12, 2016
asfgit pushed a commit that referenced this pull request May 12, 2016
Disable NioTestHistorically NioTest has caused issue in CI environment and several developer machines due to network requirements which could be disabled by firewall or security enforcers such as selinux. This disables the test once again using a historic commit 881a6e1
Signed-off-by: Rohit Yadav <rohit.yada

To build and just run this test: mvn clean install -pl utils -Dtest=NioTest

* pr/1538:
  utils: Disable NioTest

Signed-off-by: Will Stevens <[email protected]>
@swill
Copy link
Contributor

swill commented May 12, 2016

I have forced pushed this because it is blocking other PRs that are only pending Jenkins and Travis and since we are so close to freeze date, I don't want stuff to not get in because of stupid stuff like an unrelated test failing...

asfgit pushed a commit that referenced this pull request May 16, 2016
This reverts commit f88cb88, reversing
changes made to 688522e.

This was reverted because it seemed to be related to an issue
when doing a DeployDC, causing an `addHost` error.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants