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

[OSJC-256] DefaultClient#equal <true> if authstrategies match in username #164

Merged
merged 1 commit into from
May 24, 2016

Conversation

adietish
Copy link
Member

@adietish adietish commented May 24, 2016

No description provided.

@adietish adietish changed the title [OSJC-256] DefaultClient#equal <true> if authstrategies match in user… [OSJC-256] DefaultClient#equal <true> if authstrategies match in username May 24, 2016
@adietish
Copy link
Member Author

[test]

@adietish
Copy link
Member Author

@jcantrill dare to review?

@adietish
Copy link
Member Author

[test]

@adietish
Copy link
Member Author

weird, travis fails to build complainig that it cannot find the branch OSJC-256...

@adietish adietish force-pushed the OSJC-256 branch 2 times, most recently from e28f0cf to 5a79081 Compare May 24, 2016 16:35
assertThat(tokenClient).isNotEqualTo(basicAuthClient);
}

@Test
public void client_should_equal_client_with_same_TokenAuthStrategy_with_different_token() throws Exception {
Copy link
Contributor

Choose a reason for hiding this comment

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

@adietish Looks like you lost a test

Copy link
Member Author

Choose a reason for hiding this comment

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

@jcantrill nope, that's on purpose. See the method name, it says that 2 clients with different strategies should not be equal, that's exactly the opposite of what I wanted to change, so the test has to disappear.

@jcantrill
Copy link
Contributor

LGTM. Per our conversation, I think equality should be obtained when the username and server match. Other then verifying the 'lost' test I think this is ok to merge.

@openshift-bot
Copy link
Contributor

Evaluated for javaclient test up to 40935ea

@openshift-bot
Copy link
Contributor

@adietish
Copy link
Member Author

[merge]

@openshift-bot
Copy link
Contributor

openshift-bot commented May 24, 2016

Java Client Merge Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test-openshift-restclient-java/125/)

@openshift-bot
Copy link
Contributor

Evaluated for javaclient merge up to 40935ea

@openshift-bot openshift-bot merged commit 346a85a into openshift:master May 24, 2016
@adietish adietish deleted the OSJC-256 branch September 20, 2016 08:11
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

3 participants