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

Assertion is not done against isRepresentativeRun when using median-run #1064

Open
b3nk3 opened this issue Jul 26, 2024 · 6 comments
Open

Assertion is not done against isRepresentativeRun when using median-run #1064

b3nk3 opened this issue Jul 26, 2024 · 6 comments

Comments

@b3nk3
Copy link

b3nk3 commented Jul 26, 2024

Describe the bug
When using the median-run aggregation method I'd expect that assertions are done against the run, that has the property of isRepresentativeRun in the manifest.json. This does not appear to be the case.

Manifest:

{
    "url": "http://localhost:3000/",
    "isRepresentativeRun": true,
    "htmlPath": "frontend/.lighthouseci/localhost-_-2024_07_26_09_57_40.report.html",
    "jsonPath": "frontend/.lighthouseci/localhost-_-2024_07_26_09_57_40.report.json",
    "summary": {
      "performance": 0.68,
      "accessibility": 0.98,
      "best-practices": 1,
      "seo": 1
    }
  },

However, when correlating this with the assertion-resutls.json it is apparent, that a different run was used for assertion.

[
  {
    "name": "minScore",
    "expected": 0.7,
    "actual": 0.69,
    "values": [
      0.57,
      0.69,
      0.68
    ],
    "operator": ">=",
    "passed": false,
    "auditProperty": "performance",
    "auditId": "categories",
    "level": "error",
    "url": "http://localhost:3000/"
  }
]

To Reproduce
Steps to reproduce the behavior:
Use run LHCI with filesystem target, using the following assertion:

{
          matchingUrlPattern: `^${PREVIEW_URL}/$`,
          assertions: {
            'categories:performance': ['error', { minScore: 0.7, aggregationMethod: 'median-run' }],
            'categories:accessibility': ['error', { minScore: 0.9, aggregationMethod: 'median-run' }],
            'categories:best-practices': ['error', { minScore: 0.9, aggregationMethod: 'median-run' }],
            'categories:seo': ['error', { minScore: 0.9, aggregationMethod: 'median-run' }],
          },
        },

Expected behavior

Assertion should be done against the run with the isRepresentativeRun property in the manifest.json

As per the description of median-run in the Aggregation methods section of the config documentation:

Use the value of the run that was determined to be "most representative" of all runs...

Environment (please complete the following information):

@b3nk3 b3nk3 changed the title Assertion is not done agains isRepresentativeRun when using median-run Assertion is not done against isRepresentativeRun when using median-run Jul 26, 2024
@b3nk3
Copy link
Author

b3nk3 commented Aug 9, 2024

pinging for an update...

@b3nk3
Copy link
Author

b3nk3 commented Sep 2, 2024

Still looking for an answer on this

@paulirish
Copy link
Member

@b3nk3 Sorry Ben.. It's unlikely we'll be able to dig into this anytime soon. Very happy to review PRs, though. Apologies we can't investigate ourselves.

@b3nk3
Copy link
Author

b3nk3 commented Sep 30, 2024

@paulirish thanks for coming back - I'll see if I can dig this out myself - any pointers where I could start is welcome...

@paulirish
Copy link
Member

paulirish commented Sep 30, 2024

any pointers

const lhrsToUseForAudit = options.aggregationMethod === 'median-run' ? medianLhrs : lhrs;
const auditResults = lhrsToUseForAudit.map(lhr => lhr.audits[auditId]);
const assertionResults = getAssertionResultsForAudit(
auditId,
auditProperty,
auditResults,
options,
lhrs
);

hmm.. maybe because getAssertionResultsForAudit gets passed all LHRs and not just the lhrsToUseForAudit ? Not really sure. (I'm unfamiliar with this code :)

@b3nk3
Copy link
Author

b3nk3 commented Oct 1, 2024

Thanks - I'll try to spend some time figuring this out.

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

No branches or pull requests

2 participants