Skip to content

Convert lettuce video tests specifically using youtube stub - #3492

Merged
muhammad-ammar merged 1 commit into
masterfrom
ammar/bok-choy-youtube-stub-video-tests
May 7, 2014
Merged

Convert lettuce video tests specifically using youtube stub#3492
muhammad-ammar merged 1 commit into
masterfrom
ammar/bok-choy-youtube-stub-video-tests

Conversation

@muhammad-ammar

Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It seems to me you shouldn't need time.sleep() since self.navigate_to_video calls self.video.wait_for_video_player_render(). What is the reasoning for using this? I may be misinterpreting.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There is a timeout We wait a bit and see if YouTube responds. If not, then the switching to HTML5 happens. So that's 1.5 seconds. We wait for 3 second in the test to be completely sure that the switching finished.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This function should be taking care of waiting for the video to load so that we don't need to rely on time.sleep in the test.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@muhammad-ammar Yes, it should works well without time.sleep(3.0). _wait_for_element, that is used inside wait_for_video_player_render, will wait for this 3 seconds under the hood (EmptyPromise(...,timeout=200).fulfill()).

@jzoldak

jzoldak commented May 2, 2014

Copy link
Copy Markdown
Contributor

Please add to this PR the changes to video.feature that will remove the equivalent lettuce tests.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

    Scenario: Video is rendered in Youtube mode when the YouTube Server responds quickly

@polesye

polesye commented May 5, 2014

Copy link
Copy Markdown
Contributor

@olmar please review this PR.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is it possible to import and use VIDEO_SOURCE_PORT and YOUTUBE_STUB_PORT values from settings?
like here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As per my understanding its not possible.

@olmar

olmar commented May 6, 2014

Copy link
Copy Markdown
Contributor

👍

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

self.youtube_configuration.update({
  'time_to_response': 2.0,
  'youtube_api_blocked': True,
});

@polesye

polesye commented May 6, 2014

Copy link
Copy Markdown
Contributor

I left 2 small comments. Anyway, it looks good to merge 👍 .

muhammad-ammar added a commit that referenced this pull request May 7, 2014
…ests

Convert lettuce video tests specifically using youtube stub
@muhammad-ammar
muhammad-ammar merged commit ca0315e into master May 7, 2014
@muhammad-ammar
muhammad-ammar deleted the ammar/bok-choy-youtube-stub-video-tests branch May 7, 2014 07:49
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.

6 participants