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 mock response for demo test if Vimeo is down #10720

Merged
merged 1 commit into from Oct 19, 2018

Conversation

notnownikki
Copy link
Member

@notnownikki notnownikki commented Oct 18, 2018

Description

This makes the demo test more resilient if Vimeo is having problems.

We want at least one test that does a full e2e test with a working embed provider,
but if that provider is down or having problems, we don't want to hold up development.

This PR checks if the embed response has been successful or not, and if it has not
a mock response is used.

How has this been tested?

Run the e2e tests, they should pass.
Edit your hosts file and set vimeo.com to 127.0.1.1 and run the e2e tests again. They should still pass.

Types of changes

Bug fix

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.

@aduth
Copy link
Member

aduth commented Oct 18, 2018

Will this help in allowing us to re-commit #10682 ?

@aduth aduth added the [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. label Oct 18, 2018
@notnownikki
Copy link
Member Author

Yes, if the keyboard demo test was fully mocked, and this test used the mock when the real request fails,we should be good.

@aduth
Copy link
Member

aduth commented Oct 18, 2018

Does that mean we'd need to duplicate this into the other suite? Can we generalize this to mock third-party requests across all tests, i.e. in support/setup-test-framework.js?

We want at least one test that does a full e2e test with a working embed provider,
but if that provider is down or having problems, we don't want to hold up development.

My understanding of the need is for verification of the aspect ratio detection. Looking at the implementation here, couldn't it be enough to use the mock markup in all cases?

It really doesn't seem like we should ever need to communicate with a third-party to verify the intended behavior.

@notnownikki
Copy link
Member Author

It's not enough to use the mock if we want to be 100% sure. For example, YouTube changed embed dimensions not long ago to support different aspect ratios, and we need to catch if Vimeo do something similar. The video were using is 16:9, so the risk is very very small... I'll leave it up to your judgement, I'd feel more comfortable using the actual response though.

@aduth
Copy link
Member

aduth commented Oct 18, 2018

Do we really care the specific dimension they're using, or that our system accommodates for whatever it might be?

The way I see it, our tests should target the latter. Whether YouTube returns a height of 800 or 200 pixels shouldn't really matter.

@notnownikki
Copy link
Member Author

notnownikki commented Oct 18, 2018 via email

@aduth
Copy link
Member

aduth commented Oct 18, 2018

Gotcha, that makes sense, though as something specific for our demo test. Separately, we still need something to neutralize external requests in other test cases (including #10682). I suppose that could be considered outside the scope of this pull request.

@notnownikki notnownikki merged commit 48c1209 into master Oct 19, 2018
@notnownikki notnownikki deleted the update/demo-test-mock-if-vimeo-fails branch October 19, 2018 13:01
antpb pushed a commit to antpb/gutenberg that referenced this pull request Oct 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants