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

✨🏗 Add expectAsyncConsoleError, which enables tests to check for async errors #15621

Merged
merged 1 commit into from
May 26, 2018
Merged

✨🏗 Add expectAsyncConsoleError, which enables tests to check for async errors #15621

merged 1 commit into from
May 26, 2018

Conversation

rsimha
Copy link
Contributor

@rsimha rsimha commented May 26, 2018

The allowConsoleError construct lets you capture synchronous calls to console.error in a test, and indicate that they are expected and that the test shouldn't fail. However, when test code results in asynchronous errors, there's no good way to use allowConsoleError as written today.

This PR does the following:

  • Introduces a new construct called expectAsyncConsoleError, which lets you specify that a given asynchronous console.error is expected at some point during the test.
    • When the error eventually appears in the test, it is ignored.
    • If the expected error doesn't appear by the time the test ends, an error is thrown.
  • Adds an expectAsyncConsoleError annotation to all tests in extensions/amp-analytics/0.1/test/test-amp-analytics.js that were resulting in async errors
  • Undoes the temporary workaround added by Temporarily disable console error check in test-amp-analytics.js unti… #15606

Fixes #15609
Required for #14406

@rsimha
Copy link
Contributor Author

rsimha commented May 26, 2018

/to @lannka @choumx

Copy link
Contributor

@lannka lannka left a comment

Choose a reason for hiding this comment

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

Awesome! Thx!

@rsimha rsimha merged commit cbf3929 into ampproject:master May 26, 2018
@rsimha rsimha deleted the 2018-05-25-AsyncErrors branch May 26, 2018 03:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants