Improve tests about ember-youtube HOT 5 CLOSED

That's a good plan. Thank you again! I'll leave this issue open until we have at least a basic test that verifies that it can play a YouTube video.

from ember-youtube.

The problem

I've run into a problem with this piece of code. I was testing my own component that uses ember-youtube inside of it and the test below is about changing the currentVideoId for the player.

test('it should change video by click on the next video button', function (assert) {
  assert.expect(1);

  this.render(hbs`
    {{#v-video-player 
       isOpen=true
       video=videos
       currentVideoId=currentVideoId}}
    {{/v-video-player}}
  `);

  this.$('.js-v-video-player__next').click();
  assert.equal(this.get('currentVideoId'), this.get('videos')[1].videoId);
});

.js-v-video-player__next here is a button that switches currentVideoId to the next video in the list of videos.

I've tried even this option:

return wait().then(() => {
  this.$('.js-v-video-player__next').click();
  assert.equal(this.get('currentVideoId'), this.get('videos')[1].videoId);
});

but it works not all the time. Sometimes it fails and I haven't realized what it depends on.
So, logically, firstly it starts initializing the player, but it doesn't finish before ytid changing and trying to run player initializing one more time.

The Solution

What about to save a promise from loadAndCreatePlayer to a property and return it every time when loadAndCreatePlayer is called, but the promise hasn't finished. I'd be looking like this:

     loadAndCreatePlayer() {
		let isRunning = this.get('loadAndCreatePlayerIsRunning');
		if (isRunning) {
			// some ember-concurrency would be nice here
			return this.get('loadAndCreatePlayerIsRunning');
		}
		const promise = new RSVP.Promise((resolve, reject) => {
			this.loadYouTubeApi().then(() => {
				this.createPlayer().then(player => {
					this.setProperties({
						player,
						playerState: 'ready'
					});
					this.sendAction('playerCreated', player);
					this.set('loadAndCreatePlayerIsRunning', false);
					resolve();
				})
				.catch(err => {
					if (this.get('showDebug')) {
						Ember.debug(err);
					}
					reject(err);
				});
			});
		});
    this.set('loadAndCreatePlayerIsRunning', promise);
    // The `wait` helper waits for this run loop,
		// but not the above promise, which is what i want.
		if (Ember.testing) {
			run.later(() => {}, 5000);
		}
		return promise;
	},

In that way, tests pass every time. I'm gonna test this thing a bit more and send a PR with this editings if it's needed.

@oskarrough, what do you think?

from ember-youtube.

@lancedikson very happy that you're looking into it. I think that changes makes sense. Especially when it makes it testable.

I actually started refactoring it with ember-concurrency here https://github.com/oskarrough/ember-youtube/blob/develop/addon/components/ember-youtube.js#L28 to simplify exactly this logic but didn't get to finish it up. Also ended up rewriting much more than I should have in that branch so will have to try again.

from ember-youtube.

@oskarrough great! I guess we still could have my proposed fix as a hotfix and then you can release ember-concurrency implementation whenever it's done, right? I will send a PR today if you agree :)

from ember-youtube.

@lancedikson I think this is now finally fixed… with ember 3.0 there's a test helper waitFor that can wait for an element to appear. Very handy.

from ember-youtube.