-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Fix Github tests #1426
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
Fix Github tests #1426
Conversation
Change-Id: Ic2cd64fb53cd5b932c8b2920d2bfb084e70cc48e
*/ | ||
public static final Set<String> SUPPORTED_PROVIDERS = | ||
Collections.unmodifiableSet(new HashSet<>(Arrays.asList( | ||
GoogleAuthProvider.PROVIDER_ID, | ||
FacebookAuthProvider.PROVIDER_ID, | ||
TwitterAuthProvider.PROVIDER_ID, | ||
GithubAuthProvider.PROVIDER_ID, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoa, that's a public API, not part of TestConstants
. 😛 I wasn't merged up with master before so I didn't see the changes. Here's how I fixed it:
Lines 74 to 76 in 98353a7
List<String> allExceptGitHub = new ArrayList<>(AuthUI.SUPPORTED_PROVIDERS); | |
allExceptGitHub.remove(GithubAuthProvider.PROVIDER_ID); | |
FlowParameters testParams = TestHelper.getFlowParameters(allExceptGitHub); |
If we did that here:
Lines 319 to 320 in 98353a7
FlowParameters testParams = TestHelper.getFlowParameters(AuthUI.SUPPORTED_PROVIDERS, /* enableAnonymousUpgrade */ true); | |
mHandler.initializeForTesting(testParams, mMockAuth, null, null); |
it should work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But yeah, I couldn't find a way to include GitHub in the tests. 😢
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh holy crap haha I did not look at my file path!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No prob 😆
Change-Id: I5e8824935940c9e1a876fea7da0969b97755730e
This fixes #1425
We can't add a dependency from
auth
toauth-github
because that would be circular.Maybe we need an
auth-common
module in the long term? That would be a bit unfortunate, but could fix the issue.