core(fr): convert base artifacts to gatherer artifacts#12129
Conversation
connorjclark
left a comment
There was a problem hiding this comment.
LGTM, just a couple comments. but since this is so big I think a second person should review too.
Co-authored-by: Connor Clark <cjamcl@google.com>
…e into fr_base_artifacts
|
@adamraine you interested in giving this a second look? :) |
| const {auditResults, erroredAudits, failedAudits} = getAuditsBreakdown(lhr); | ||
| // TODO(FR-COMPAT): This assertion can be removed when full compatibility is reached. | ||
| expect(auditResults.length).toMatchInlineSnapshot(`65`); | ||
| expect(auditResults.length).toMatchInlineSnapshot(`67`); |
There was a problem hiding this comment.
Weren't there 3 gatherers added that can do snapshot mode? InstallabilityErrors, Stacks, and WebAppManifest?
There was a problem hiding this comment.
yes great catch! turns out quite a few of the artifact ids were misspelled. we have 7 more artifacts now 🎉
There was a problem hiding this comment.
I added typechecking to the new default-config that will ensure all artifact IDs used in strings are typechecked against our usages
| const {auditResults, erroredAudits, failedAudits} = getAuditsBreakdown(lhr); | ||
| // TODO(FR-COMPAT): This assertion can be removed when full compatibility is reached. | ||
| expect(auditResults.length).toMatchInlineSnapshot(`67`); | ||
| expect(auditResults.length).toMatchInlineSnapshot(`72`); |
There was a problem hiding this comment.
There were 4 misspellings but this jumped from 67 to 72. Is that because more than 1 audit use the same artifact?
Summary
Converts most of the base artifacts so that they're runnable in fraggle rock as regular gatherers, but preserves the current base behavior in the legacy path as well.
environment.jsoff of driver.urlas a shared property of the FRTransitionalContextRelated Issues/PRs
ref #11313