Skip to content

Add more integration tests for amp-form#7525

Merged
mkhatib merged 4 commits intoampproject:masterfrom
mkhatib:integration-tests-forms
Feb 15, 2017
Merged

Add more integration tests for amp-form#7525
mkhatib merged 4 commits intoampproject:masterfrom
mkhatib:integration-tests-forms

Conversation

@mkhatib
Copy link
Copy Markdown
Contributor

@mkhatib mkhatib commented Feb 14, 2017

Closes #7154

actionXhr: baseUrl + '/form/post',
on: 'submit:sameform.submit',
});
const ampForm = new AmpForm(form);
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.

I see AmpForm constructor takes in an element and an id? Should we add an id 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.

Done

' {{#interests}}{{title}} {{/interests}}.',
errorTemplate: 'Should not render this.',
});
new AmpForm(form);
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.

same

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.

Done

new AmpForm(form);
const errors = [];
const realSetTimeout = window.setTimeout;
sandbox.stub(window, 'setTimeout', (callback, delay) => {
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.

I don't quite get why are we stubing window.setTimeout here. Shouldn't the callback takes care of error handling?

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.

Ah yeah, I am doing this because of the rethrowAsync call inside the catch callback of the fetch call. The only way I could think of catching the async error is through stubbing timeout and forcing a try-catch clause to avoid the tests flaking because of async-thrown error.

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.

Then a brief description would be helpful here i think.

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.

Done

@mkhatib
Copy link
Copy Markdown
Contributor Author

mkhatib commented Feb 14, 2017

PTAL 👀

new AmpForm(form);
const errors = [];
const realSetTimeout = window.setTimeout;
sandbox.stub(window, 'setTimeout', (callback, delay) => {
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.

Then a brief description would be helpful here i think.

@mkhatib mkhatib merged commit b3df5cc into ampproject:master Feb 15, 2017
@mkhatib mkhatib deleted the integration-tests-forms branch February 15, 2017 20:27
mrjoro pushed a commit to mrjoro/amphtml that referenced this pull request Apr 28, 2017
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.

2 participants