Skip to content

Commit

Permalink
Fix flaky test in test-integration-form.js (#10273)
Browse files Browse the repository at this point in the history
  • Loading branch information
William Chou authored Jul 6, 2017
1 parent b8249e8 commit ee2c0bc
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 14 deletions.
7 changes: 4 additions & 3 deletions extensions/amp-form/0.1/amp-form.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

import {ActionTrust} from '../../../src/action-trust';
import {FormEvents} from './form-events';
import {installFormProxy} from './form-proxy';
import {triggerAnalyticsEvent} from '../../../src/analytics';
import {createCustomEvent} from '../../../src/event-helper';
Expand Down Expand Up @@ -702,12 +703,12 @@ export class AmpForm {
rendered.id = messageId;
rendered.setAttribute('i-amphtml-rendered', '');
container.appendChild(rendered);
const templatedEvent = createCustomEvent(
const renderedEvent = createCustomEvent(
this.win_,
AmpEvents.TEMPLATE_RENDERED,
/* detail */ null,
{bubbles: true});
container.dispatchEvent(templatedEvent);
container.dispatchEvent(renderedEvent);
});
} else {
// TODO(vializ): This is to let AMP know that the AMP elements inside
Expand Down Expand Up @@ -920,7 +921,7 @@ export class AmpFormService {
this.whenInitialized_.then(() => {
const win = ampdoc.win;
const event = createCustomEvent(
win, 'amp:form-service:initialize', null, {bubbles: true});
win, FormEvents.SERVICE_INIT, null, {bubbles: true});
win.dispatchEvent(event);
});
}
Expand Down
21 changes: 21 additions & 0 deletions extensions/amp-form/0.1/form-events.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/**
* Copyright 2017 The AMP HTML Authors. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS-IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

/** @enum {string} */
export const FormEvents = {
// Dispatched by the window when AmpFormService initializes.
SERVICE_INIT: 'amp:form-service:initialize',
};
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,6 @@ describes.realWin('AmpForm Integration', {
const ampForm = new AmpForm(form, 'form1');
const fetchSpy = sandbox.spy(ampForm.xhr_, 'fetch');
const fetch = poll('submit request sent', () => fetchSpy.returnValues[0]);
const layout = poll('amp-img layout completes',
() => form.querySelector('amp-img img'));

form.dispatchEvent(new Event('submit'));
return fetch.then(() => {
Expand All @@ -306,8 +304,10 @@ describes.realWin('AmpForm Integration', {
const rendered = form.querySelectorAll('[i-amphtml-rendered]');
expect(rendered.length).to.equal(0);

// Any amp elements inside the message should be layed out
return layout.then(img => {
// Any amp elements inside the message should be layed out.
const layout = listenOncePromise(form, AmpEvents.LOAD_START);
return layout.then(() => {
const img = form.querySelector('amp-img img');
expect(img.src).to.contain('/examples/img/ampicon.png');
});
});
Expand Down
3 changes: 2 additions & 1 deletion test/integration/test-amp-bind.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
import {AmpEvents} from '../../src/amp-events';
import {BindEvents} from '../../extensions/amp-bind/0.1/bind-events';
import {FormEvents} from '../../extensions/amp-form/0.1/form-events';
import {createFixtureIframe} from '../../testing/iframe';
import {batchedXhrFor} from '../../src/services';
import * as sinon from 'sinon';
Expand Down Expand Up @@ -98,7 +99,7 @@ describe.configure().skipSauceLabs().run('amp-bind', function() {
// <form> is not an AMP element.
return setupWithFixture('test/fixtures/bind-form.html', 0)
// Wait for AmpFormService to register <form> elements.
.then(() => fixture.awaitEvent('amp:form-service:initialize', 1));
.then(() => fixture.awaitEvent(FormEvents.SERVICE_INIT, 1));
});

it('should NOT allow invalid bindings or values', () => {
Expand Down
13 changes: 7 additions & 6 deletions testing/iframe.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@
* limitations under the License.
*/

import {AmpEvents} from '../src/amp-events';
import {BindEvents} from '../extensions/amp-bind/0.1/bind-events';
import {FakeLocation} from './fake-dom';
import {FormEvents} from '../extensions/amp-form/0.1/form-events';
import {ampdocServiceFor} from '../src/ampdoc';
import {cssText} from '../build/css';
import {deserializeMessage, isAmpMessage} from '../src/3p-frame-messaging';
Expand All @@ -30,8 +33,6 @@ import {installDocService} from '../src/service/ampdoc-impl';
import {installExtensionsService} from '../src/service/extensions-impl';
import {installStyles} from '../src/style-installer';
import {resourcesForDoc} from '../src/services';
import {AmpEvents} from '../src/amp-events';
import {BindEvents} from '../extensions/amp-bind/0.1/bind-events';

let iframeCount = 0;

Expand Down Expand Up @@ -66,14 +67,14 @@ export function createFixtureIframe(fixture, initialIframeHeight, opt_beforeLoad
return new Promise((resolve, reject) => {
// Counts the supported custom events.
const events = {
'amp:form-service:initialize': 0,
[AmpEvents.ATTACHED]: 0,
[BindEvents.INITIALIZE]: 0,
[BindEvents.SET_STATE]: 0,
[BindEvents.RESCAN_TEMPLATE]: 0,
[AmpEvents.ERROR]: 0,
[AmpEvents.LOAD_START]: 0,
[AmpEvents.STUBBED]: 0,
[BindEvents.INITIALIZE]: 0,
[BindEvents.SET_STATE]: 0,
[BindEvents.RESCAN_TEMPLATE]: 0,
[FormEvents.SERVICE_INIT]: 0,
};
const messages = [];
let html = __html__[fixture];
Expand Down

0 comments on commit ee2c0bc

Please sign in to comment.