Skip to content

Commit

Permalink
core: remove ScriptElements artifact (#15879)
Browse files Browse the repository at this point in the history
  • Loading branch information
adamraine authored Mar 21, 2024
1 parent 9c8d0cf commit b64619a
Show file tree
Hide file tree
Showing 13 changed files with 16 additions and 356 deletions.
4 changes: 2 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ A PR adding or changing a gatherer almost always needs to include the following:
1. **Golden artifacts**: `sample_v2.json` is generated from a set of artifacts that come from running LH against `dbw_tester.html`. Those artifacts likely need to be updated after gatherer changes with `yarn update:sample-artifacts`, but limit to just the artifact being altered if possible. For example:

```sh
# update just the ScriptElements artifact
yarn update:sample-artifacts ScriptElements
# update just the Scripts artifact
yarn update:sample-artifacts Scripts
```

This command works for updating `yarn update:sample-artifacts DevtoolsLog` or `Trace` as well, but the resulting `sample_v2.json` churn may be extensive and you might be better off editing manually.
Expand Down
67 changes: 0 additions & 67 deletions cli/test/smokehouse/test-definitions/byte-efficiency.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ const config = {
// unsized-images is not a byte-efficiency audit but can easily leverage the variety of images present in
// byte-efficiency tests & thus makes sense to test together.
'unsized-images',
'script-elements-test-audit',
],
throttlingMethod: 'devtools',
},
Expand All @@ -40,7 +39,6 @@ const config = {
// Lower the threshold so we don't need huge resources to make a test.
unusedThreshold: 2000,
}},
'script-elements-test-audit',
],
};

Expand All @@ -50,71 +48,6 @@ const config = {
*/
const expectations = {
artifacts: {
ScriptElements: [
{
type: null,
src: null,
async: false,
defer: false,
source: 'head',
// Only do a single assertion for `devtoolsNodePath`: this can be flaky for elements
// deep in the DOM, and the sample LHR test has plenty of places that would catch
// a regression in `devtoolsNodePath` calculation. Keep just one for the benefit
// of other smoke test runners.
node: {
devtoolsNodePath: '2,HTML,0,HEAD,3,SCRIPT',
},
},
{
type: 'application/javascript',
src: 'http://localhost:10200/byte-efficiency/script.js',
async: false,
defer: false,
source: 'head',
},
{
type: null,
src: 'http://localhost:10200/byte-efficiency/bundle.js',
async: false,
defer: false,
source: 'head',
},
{
type: null,
src: null,
async: false,
defer: false,
source: 'body',
},
{
type: null,
src: null,
async: false,
defer: false,
source: 'body',
},
{
type: null,
src: null,
async: false,
defer: false,
source: 'body',
},
{
type: null,
src: null,
async: false,
defer: false,
source: 'body',
},
{
type: null,
src: 'http://localhost:10200/byte-efficiency/delay-complete.js?delay=8000',
async: true,
defer: false,
source: 'body',
},
],
Scripts: {
_includes: [
{
Expand Down
3 changes: 0 additions & 3 deletions cli/test/smokehouse/test-definitions/oopif-requests.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,12 @@ const config = {
title: 'Performance',
auditRefs: [
{id: 'oopif-iframe-test-audit', weight: 0},
{id: 'script-elements-test-audit', weight: 0},
],
},
},
audits: [
// Include an audit that *forces* the IFrameElements artifact to be used for our test.
{path: 'oopif-iframe-test-audit'},
{path: 'script-elements-test-audit'},
],
settings: {
// This test runs in CI and hits the outside network of a live site.
Expand Down Expand Up @@ -86,7 +84,6 @@ const expectations = {
isPositionFixed: false,
},
],
ScriptElements: [],
Scripts: [],
},
};
Expand Down
24 changes: 2 additions & 22 deletions cli/test/smokehouse/test-definitions/oopif-scripts.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,12 @@ const config = {
title: 'Performance',
auditRefs: [
{id: 'oopif-iframe-test-audit', weight: 0},
{id: 'script-elements-test-audit', weight: 0},
],
},
},
audits: [
// Include an audit that *forces* the IFrameElements artifact to be used for our test.
{path: 'oopif-iframe-test-audit'},
{path: 'script-elements-test-audit'},
],
settings: {
// This test runs in CI and hits the outside network of a live site.
Expand Down Expand Up @@ -99,27 +97,9 @@ const expectations = {
isPositionFixed: true,
},
],
// Only `:10200/oopif-simple-page.html`'s inclusion of `simple-script.js` shows here.
// Only `:10200/oopif-simple-page.html`'s inclusion of `simple-script.js` shows here,
// as well as inline scripts of the iframe.
// All other scripts are filtered out because of our "OOPIF" filter.
ScriptElements: [
{
src: 'http://localhost:10200/simple-script.js',
source: 'network',
},
{
// Worker requests emitted on the worker's parent target since M123.
_minChromiumVersion: '123',
src: 'http://localhost:10200/simple-worker.mjs',
source: 'network',
},
{
// Worker requests emitted on the worker's parent target since M123.
_minChromiumVersion: '123',
src: 'http://localhost:10200/simple-worker.js',
source: 'network',
},
],
// Same here, except we get inline scripts of the iframe.
Scripts: {
_includes: [
{
Expand Down
29 changes: 0 additions & 29 deletions core/audits/script-elements-test-audit.js

This file was deleted.

1 change: 0 additions & 1 deletion core/config/default-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,6 @@ const defaultConfig = {
{id: 'ResponseCompression', gatherer: 'dobetterweb/response-compression'},
{id: 'RobotsTxt', gatherer: 'seo/robots-txt'},
{id: 'ServiceWorker', gatherer: 'service-worker'},
{id: 'ScriptElements', gatherer: 'script-elements'},
{id: 'Scripts', gatherer: 'scripts'},
{id: 'SourceMaps', gatherer: 'source-maps'},
{id: 'Stacks', gatherer: 'stacks'},
Expand Down
100 changes: 0 additions & 100 deletions core/gather/gatherers/script-elements.js

This file was deleted.

2 changes: 1 addition & 1 deletion core/test/config/config-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ describe('getConfigDisplayString', () => {
passes: [{
passName: 'defaultPass',
gatherers: [
{path: 'script-elements'},
{path: 'scripts'},
],
}],
audits: [
Expand Down
Loading

0 comments on commit b64619a

Please sign in to comment.