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

Fix and re-enable webpurify tests #21878

Merged
merged 2 commits into from Apr 17, 2018
Merged

Fix and re-enable webpurify tests #21878

merged 2 commits into from Apr 17, 2018

Conversation

islemaster
Copy link
Contributor

@islemaster islemaster commented Apr 16, 2018

The WebPurify unit tests have been frequently failing on Circle CI lately.

While I explored changes to the WebPurify component recently, neither the source file, nor the test file, nor the VCR files have been touched since October.

VCR debug logging revealed the issue.

[VCR::Configuration] before_playback: replacing "<ENDPOINT>" with "https://region.api.cognitive.microsoft.com/contentmoderator"
[VCR::Configuration] before_playback: replacing "<API_KEY>" with "fakekey"
[VCR::Configuration] before_playback: replacing "<API_KEY>" with "mocksecret"

Ends up trying to match fakekey when it should be matching mocksecret:

[get http://api1.webpurify.com/services/rest/?api_key=fakekey&format=json&lang=en&method=webpurify.live.return&text=schei%C3%9Fe]

Meaning I actually broke the WebPurify tests by adding these Azure tests.

The root of the problem is that these VCR replacements share a global namespace, and there's a collision in the <API_KEY> replacement. I think these tests would pass if the random test ordering loaded the mocksecret replacement first.

The fix is to give the Azure tests a unique secret replacement, like <AZURE_API_KEY>, and regenerate those VCR files.

@islemaster islemaster changed the title Fix webpurify tests Fix and re-enable webpurify tests Apr 17, 2018
Copy link
Member

@davidsbailey davidsbailey left a comment

Choose a reason for hiding this comment

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

Nice find, nice fix!

@islemaster islemaster merged commit b5c06b7 into staging Apr 17, 2018
@islemaster islemaster deleted the fix-webpurify-tests branch April 17, 2018 01:46
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

2 participants