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

Use a better solution to fix the ID test that doesnt work in local #16536

Merged
merged 5 commits into from Apr 25, 2017

Conversation

DiegoVazquezNanini
Copy link
Contributor

This PR fix an issue with some ID tests that fail if you run the tests locally.
@jfsoul @markjamesbutler @TBonnin

Copy link
Contributor

@TBonnin TBonnin left a comment

Choose a reason for hiding this comment

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

Something like this https://gist.github.com/TBonnin/7b11368f841796dbcb286c5fe4e12cec, in additions to your changes of IdApi and IdApiClient, would be cleaner.

Then in all the tests involving identity config you can provide your own implementation of the IdConfig trait, the same way we are providing TestWsClient for instance.

@DiegoVazquezNanini
Copy link
Contributor Author

@TBonnin please let me know what do you think now and please let me know if you have further suggestions.

val accountDeletionApiKey: String = ???
val url: String = ???
val oauthUrl: String = ???
val domain: String = ???
Copy link
Contributor

Choose a reason for hiding this comment

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

You are gonna need real values here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeap, I'm working on a fix now.

@PRBuilds
Copy link

PR build results:

screenshots
mobile.pngwide.pngtablet.pngdesktop.png

exceptions (0)
thrown-exceptions.js

webpagetest (2)
performanceComparisonSummary.txt

-automated message

@DiegoVazquezNanini
Copy link
Contributor Author

Thanks for the review @TBonnin

@DiegoVazquezNanini DiegoVazquezNanini merged commit 787e4be into master Apr 25, 2017
@DiegoVazquezNanini DiegoVazquezNanini deleted the dv-fix-identity-test-for-dev-take2 branch April 25, 2017 10:44
@prout-bot
Copy link
Collaborator

Seen on PROD (merged by @DiegoVazquezNanini 15 minutes and 5 seconds ago) Please check your changes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants