tests(i18n): only accept IcuMessages in toBeDisplayString #12487
Conversation
| @@ -167,7 +167,7 @@ describe('PWA: webapp install banner audit', () => { | |||
| return InstallableManifestAudit.audit(artifacts, context).then(result => { | |||
| assert.strictEqual(result.score, 0); | |||
| const items = result.details.items; | |||
| expect(items[0].reason).toBeDisplayString(/unexpected arguments/); | |||
| expect(items[0].reason).toMatch(/unexpected arguments/); | |||
There was a problem hiding this comment.
these two are kind of weird because we're putting them in the audit table, but really they're messages to us that the protocol has diverged from what we expect, so it seems better to leave them untranslated (maybe they should be just Sentry errors and test assertions that they don't occur in CI and at least leave out the "unexpected arguments" part for users?)
| @@ -84,7 +84,7 @@ describe('Security: HTTPS audit', () => { | |||
| // Unknown blocked resolution string is used as fallback. | |||
| expect(result.details.items[2]).toMatchObject({ | |||
| url: 'http://localhost/image2.jpeg', | |||
| resolution: expect.toBeDisplayString('MixedContentBlockedLOL'), | |||
| resolution: 'MixedContentBlockedLOL', | |||
There was a problem hiding this comment.
this is testing that it falls back to just using the protocol-provided string when it's a string that we don't recognize, so of course it's not localized.
| @@ -103,8 +102,7 @@ describe('Module Tests', function() { | |||
| try { | |||
| await lighthouse('file:///a/fake/index.html', {}, {}); | |||
| } catch (err) { | |||
| expect(err.friendlyMessage) | |||
| .toBeDisplayString('The URL you have provided appears to be invalid.'); | |||
| expect(err.friendlyMessage).toBe('The URL you have provided appears to be invalid.'); | |||
There was a problem hiding this comment.
runner localizes these internally (and there's a test for that), so they aren't LH.IcuMessages by the time index.js sees them
| @@ -821,7 +821,7 @@ describe('Runner', () => { | |||
|
|
|||
| // But top-level runtimeError is the pageLoadError. | |||
| expect(lhr.runtimeError.code).toEqual(LHError.errors.PAGE_HUNG.code); | |||
| expect(lhr.runtimeError.message).toBeDisplayString(/because the page stopped responding/); | |||
| expect(lhr.runtimeError.message).toMatch(/because the page stopped responding/); | |||
There was a problem hiding this comment.
LHRs are already localized when returned from Runner.run, so can just match the strings directly
Adds an assertion to
expect.toBeDisplayStringthat the passed-in value is anLH.IcuMessageand not a matching string.Originally
expect.toBeDisplayStringwas intended to support eitherstringorLH.IcuMessagesince most of the localized LHR properties end up being a union of the two. However, since then, it's clear that our tests always know if a tested property is going to be localizable or not so it's better to be specific. Doing it this way also avoids the potential to assert that a value is a certain string once localized, but miss the fact that it's just the english string that won't be localized (luckily we don't appear to have that problem anywhere).Also fixes a few places where these were incorrectly being asserted, mostly on fallbacks that occur specifically when i18n is failing for that audit and on values outside runner, which have usually already been localized.