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

E2E Tests: Remove unnecessary font warning exception #18754

Merged
merged 1 commit into from
Nov 26, 2019

Conversation

aduth
Copy link
Member

@aduth aduth commented Nov 26, 2019

Previously: #15502
See: https://core.trac.wordpress.org/ticket/47183

This pull request seeks to remove an exception added to the E2E test console log monitoring. In the initial release of WordPress 5.2, a warning could occur due to a duplicate font-face declaration in the Dashicons stylesheet. This has since been fixed, and was included in the WordPress 5.2.1 maintenance release.

Testing Instructions:

The Travis build should not fail.

@aduth aduth added [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. [Type] Code Quality Issues or PRs that relate to code quality [Package] E2E Tests /packages/e2e-tests labels Nov 26, 2019
@aduth aduth requested a review from desrosj November 26, 2019 13:26
Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

Nice to see it was fixed upstream 👍

@aduth aduth merged commit f3d1525 into master Nov 26, 2019
@aduth aduth deleted the remove/e2e-font-warning-exception branch November 26, 2019 15:11
@youknowriad youknowriad added this to the Gutenberg 7.1 milestone Dec 9, 2019
@ellatrix
Copy link
Member

@aduth I think we need to revert this. I see the error, probably caused by an embed loading fonts.

FAIL packages/e2e-tests/specs/editor/various/embedding.test.js (26.719s)
940  Embedding content
941    ✓ should render embeds in the correct state (7913ms)
942    ✕ should allow the user to convert unembeddable URLs to a paragraph with a link in it (3713ms)
943    ✓ should retry embeds that could not be embedded with trailing slashes, without the trailing slashes (3906ms)
944    ✓ should allow the user to try embedding a failed URL again (4259ms)
945    ✓ should switch to the WordPress block correctly (6358ms)
946
947  ● Embedding content › should allow the user to convert unembeddable URLs to a paragraph with a link in it
948
949    expect(jest.fn()).not.toHaveWarned(expected)
950
951    Expected mock function not to be called but it was called with:
952    ["Failed to decode downloaded font: https://fonts.gstatic.com/stats/Noto+Serif/normal/700"],["Failed to decode downloaded font: https://fonts.gstatic.com/stats/Noto+Serif/normal/700"]
953
954      at Object.expect (../jest-console/build/@wordpress/jest-console/src/index.js:34:4)
955          at runMicrotasks (<anonymous>)
956
TravisFoldPassesReporter
957...15 passing tests.
cache.2
1054store build cache
1068
1069
1070Done. Your build exited with 1.

ellatrix added a commit that referenced this pull request Dec 24, 2019
@ellatrix
Copy link
Member

I can't reproduce it locally though. The font is probably ours.

@aduth
Copy link
Member Author

aduth commented Jan 2, 2020

@aduth I think we need to revert this. I see the error, probably caused by an embed loading fonts.

I'd be okay with this as a short-term resolution, but if it's anything like the previous issue, the fact that the warning occurs is itself a bug which needs to be addressed. These warnings should not be expected.

It's hard to tell what might be causing this, given that it cannot be reproduced. In the previous issue, it was a problem of having a duplicate declaration of the font. I wonder if this is happening in the embeds test, that it might have something to do with fonts declared in the iframe? Seems strange that Chrome would consider those as "duplicate" if the frame is meant to be self-contained.

I also wonder if it's something where we need to be ignoring warnings which occur within those frames, if we're not already. Seems like we should be wanting to ignore anything within those (I think this might have been one of the motivations for mocking those embed requests?).

One thing I've observed with the embeds mocking is that it doesn't seem to be cleaning up after itself, i.e. mocks aren't removed after the tests are run. I could see how this might have some impact on it being intermittent or difficult to reproduce.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] E2E Tests /packages/e2e-tests [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants