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

Setting onlyAudits, onlyCategories and skipAudits to [] returns unexpected LHR #14986

Open
2 tasks done
benschwarz opened this issue Apr 18, 2023 · 5 comments
Open
2 tasks done

Comments

@benschwarz
Copy link
Contributor

FAQ

URL

https://example.com

What happened?

Setting onlyAudits, onlyCategories, and skipAudits to an empty array [] rather than null results in incomplete LHR JSON.

What was missing

  • lhr.categories returns {}
  • Unsure of any other omissions.

Repro script

import puppeteer from 'puppeteer'
import lighthouse from 'lighthouse'
import { writeFile } from 'node:fs'

const url = 'https://chromestatus.com/features'

// Use Puppeteer to launch headful Chrome and don't use its default 800x600 viewport.
const browser = await puppeteer.launch({
  headless: false,
  defaultViewport: null,
  ignoreDefaultArgs: ['--enable-automation']
})
const page = await browser.newPage()

const flags = { logLevel: 'debug' }
const config = {
  extends: 'lighthouse:default',
  settings: {
    skipAudits: [],
    onlyCategories: [],
    onlyAudits: []
  }
}

// Lighthouse will open the URL.
const { lhr } = await lighthouse(url, flags, config, page)

const data = new Uint8Array(Buffer.from(JSON.stringify(lhr)))
writeFile('lhr.json', data, err => {
  if (err) throw err
})

console.log(lhr.categories)

await browser.close()

What did you expect?

Expected lhr.categories to include category information.

What have you tried?

Setting to null rather than an [] results in a full LHR.

How were you running Lighthouse?

node

Lighthouse Version

10.1.0

Chrome Version

112.0.5615.121

Node Version

18.7.X

OS

Mac

Relevant log output

No response

@connorjclark
Copy link
Collaborator

This is working as expected. onlyCategories: [] is not the same as onlyCategories: null, the former is an empty set so we filter out everything, and the latter is just nothing/we ignore it. What would you suggest be different?

We could consider throwing an error at config resolution if no audits match the filter criteria.

@benschwarz
Copy link
Contributor Author

the former is an empty set so we filter out everything, and the latter is just nothing/we ignore it.

My expectation was a little bit different: Empty array = no values are set. No filtering is used.

I might the be the only person who experiences this issue (but thought I'd log, just in case), might not be a big deal, but I think it would be helpful to either throw an error, or add a log line during configuration resolution.

@connorjclark
Copy link
Collaborator

Let's throw specific errors when onlyX is an empty array (which is never useful), and a generic one when no audits are left post-filtering.

@OronW
Copy link

OronW commented May 25, 2023

@connorjclark
Hi,
I’m new to open source contribution and would like to work on this issue (this will be my second contribution to open source).
Could you please assign this issue to me?

Any pointers as to where to start looking in the code or other valuable info?
Thanks,
Oron

@OronW
Copy link

OronW commented May 27, 2023

Hi @connorjclark ,
I've added my PR with the fix for this issue (it's the second one, I had to close my first one).
I've also added unit tests and verify all tests passed locally.

As stated in my PR, this issue refers also to throwing an error for the case in which audits is left empty after filtering, however empty/missing audits is being handled in following code, and throwing an error breaks this logic.
Throwing an error for empty audits also breaks 30 tests.

I would suggest not changing any audits logic.
Please let me know if there's anything else needed for this PR.

Thanks!

@benschwarz FYI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants