Skip to content

dom: Minor test improvements#26913

Merged
mdmower merged 2 commits intoampproject:masterfrom
mdmower:pr-dom-layout
Feb 23, 2020
Merged

dom: Minor test improvements#26913
mdmower merged 2 commits intoampproject:masterfrom
mdmower:pr-dom-layout

Conversation

@mdmower
Copy link
Copy Markdown
Contributor

@mdmower mdmower commented Feb 22, 2020

Resolve sinon warning:

ERROR: '[DOM] Failed to open url on target:  _blank Error: intentional'
    The test "DOM   openWindowDialog should retry on first exception" resulted in a call to console.error. (See above line.)
    ⤷ If the error is not expected, fix the code that generated the error.
    ⤷ If the error is expected (and synchronous), use the following pattern to wrap the test code that generated the error:
        'allowConsoleError(() => { <code that generated the error> });'
    ⤷ If the error is expected (and asynchronous), use the following pattern at the top of the test:
        'expectAsyncConsoleError(<string or regex>[, <number of times the error message repeats>]);'
ERROR: 'The element did not specify a layout attribute. Check https://amp.dev/documentation/guides-and-tutorials/develop/style_and_layout/control_layout and the respective element documentation for details.'
    The test "DOM   whenUpgradeToCustomElement function should resolve if element has already upgrade" resulted in a call to console.error. (See above line.)
    ⤷ If the error is not expected, fix the code that generated the error.
    ⤷ If the error is expected (and synchronous), use the following pattern to wrap the test code that generated the error:
        'allowConsoleError(() => { <code that generated the error> });'
    ⤷ If the error is expected (and asynchronous), use the following pattern at the top of the test:
        'expectAsyncConsoleError(<string or regex>[, <number of times the error message repeats>]);'
ERROR: 'The element did not specify a layout attribute. Check https://amp.dev/documentation/guides-and-tutorials/develop/style_and_layout/control_layout and the respective element documentation for details.'
    The test "DOM   whenUpgradeToCustomElement function should resolve when element upgrade" resulted in a call to console.error. (See above line.)
    ⤷ If the error is not expected, fix the code that generated the error.
    ⤷ If the error is expected (and synchronous), use the following pattern to wrap the test code that generated the error:
        'allowConsoleError(() => { <code that generated the error> });'
    ⤷ If the error is expected (and asynchronous), use the following pattern at the top of the test:
        'expectAsyncConsoleError(<string or regex>[, <number of times the error message repeats>]);'

Full log: https://gist.github.com/mdmower/d2252c6dc06d096687c56a6368a30c41

Expect another console error.
@mdmower mdmower changed the title dom: Add layout attr where needed in unit tests dom: Minor test improvements Feb 23, 2020
@mdmower
Copy link
Copy Markdown
Contributor Author

mdmower commented Feb 23, 2020

Thanks for the review @wassgha. I added one more commit to address another sinon warning in test-dom.js. Sorry the PR wasn't complete the first time around.

@mdmower mdmower merged commit 5721bc4 into ampproject:master Feb 23, 2020
@mdmower mdmower deleted the pr-dom-layout branch February 23, 2020 19:42
robinvanopstal added a commit to jungvonmatt/amphtml that referenced this pull request Feb 24, 2020
* master: (41 commits)
  custom-element: Minor test improvements (ampproject#26923)
  amp-pixel: Minor test improvements (ampproject#26918)
  viewer: Minor test improvements (ampproject#26906)
  dom: Minor test improvements (ampproject#26913)
  amp-action: Support whitelist lookup in AmpDocShadow (ampproject#26684)
  ✨ Update amp-access-scroll (ampproject#26810)
  🚀 Remove doc css and base css from ESM build (ampproject#26889)
  📖 [amp-story-player] Initial docs (ampproject#26606)
  Amp consent restrict fullscreen prod flag (ampproject#26909)
  📖 Clarify SXG duration minimum (ampproject#26890)
  Improve test vendor requests macros (ampproject#26828)
  🚀 Move scroll left and top macros out of url-replacement-impl (ampproject#25594)
  Update consent string maximum size to 200 bytes (ampproject#26741)
  ✨[amp-story-player] Adds tap-to-next/previous story (ampproject#26865)
  update owners file with correct syntax (ampproject#26899)
  amp-sticky-ad: Fix unit test (ampproject#26855)
  Add performance metrics to README (ampproject#26891)
  🐛 Bug fix: check links test (ampproject#26739)
  ✨Idealmedia uniq ad (ampproject#25838)
  📦 Update dependency jsdom to v16.2.0 (ampproject#26591)
  ...
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.

5 participants