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

More flexible handling of categories in respect the filter options #609

Open
4 tasks
BioPhoton opened this issue Apr 9, 2024 · 3 comments
Open
4 tasks
Labels
🐛 bug something isn't working 🧩 core 🤓 UX UX improvement for CLI users

Comments

@BioPhoton
Copy link
Collaborator

BioPhoton commented Apr 9, 2024

User story

As a user of code-pushup I want to be flexible with running different set's of plugins and audits while keeping my code-pushup.config.ts file unchanged.

At the moment we support a set of filter options that affect the executed plugins and audits.

Note

I listed a couple of potential future features too to get a feeling of the bigger picture.

Category filter: (config level)

Plugin filter: (config level)

Audit filter: (plugin level)

  • packageManager in js-packages plugin - only execute NPM related audits
  • skipAudits in lighthouse plugin - exclude the listed audits from execution
  • onlyAudits in lighthouse plugin - only executes listed audits

Group filter: (plugin level)

  • onlyGroups in lighthouse plugin - only executes audits from listed groups

This filter impact directly the provided plugin configuration as well as indirectly the categories.
It changes the number of included audits, groups and sometimes even categories.

For "Plugin filter" we have logic that removes plugins as well as their related categories from the result.
For "Audit filter" it is not that easy as the plugin can not know what categories are used.

This leads to situations where we have a category configured and all it's audits excluded from the result.
As a result a error is thrown and the program exits.

Error:

ZodError: [
  {
    "code": "custom",
    "message": "The following category references need to point to an audit or group: <plugin>#<audit-slug> (audit), <plugin>#<group-slug> (group)",
    "path": []
  }
]

This is inconvenient as especially for bigger category configurations and can harm the following situations:

  • a user wants to execute only certain audits
  • a developer wants to implement audit filter for a custom plugin and cant test it
  • maitainence effort as plugins and categories are coupled in a way where it is hard to implement strict checks

Related:

Acceptance criteria

For future features like category or plugin filter we can just add another filter in the CLI middleware after the plugin filter.

For plugin level filter we should implement the following graceful handling of the above described cases:

  • every plugin has to inform the user about the usage of a filter via the logger, e.g. ui().logger.info('Lighthouse configuretion filtered the following audits: slug-1, slug-2')
  • every plugin produces a result matching the existing parser for valid plugins and its metadata
  • after the plugins are aggregated the categories are filtered and should only contain audit and group refs present in the generated AuditReports data.
  • Empty categories are filtered out

Implementation details

Extend the collect function in the core package to filter the plugin output before persisting it.

Link to the LoC:

Example implementation:

// packages/core/src/lib/implementation/collect.ts

function filterCategoriesWithOutput(pluginOutput: PluginReport[]): PluginReport[] {
   // ...
}

export async function collect(options: CollectOptions): Promise<Report> {
  const { plugins, categories } = options;
  const date = new Date().toISOString();
  const start = performance.now();
  const commit = await getLatestCommit();
  const pluginOutputs = await executePlugins(plugins, options);

  return {
    commit,
    packageName: name,
    version,
    date,
    duration: calcDuration(start),
    //                  👇  filter here
    categories: filterCategoriesWithOutput(pluginOutputs),
    plugins: pluginOutputs,
  };
}
@BioPhoton BioPhoton added 🤓 UX UX improvement for CLI users 🐛 bug something isn't working 🧩 core labels Apr 9, 2024
@matejchalk
Copy link
Collaborator

Let's add an optional isSkipped?: boolean to audit and group metadata in PluginConfig.

If there is a skipped audit/group, then the core doesn't expect it in the audit outputs from runner.

If a category references a skipped audit/group, we remove it from the refs array. If that would create an invalid refs configuration (e.g. sum of weights is now 0), then we remove category.

If an audit/group ref is removed, we log it to the console. If a category is removed, we log it to the console as well.

@hanna-skryl
Copy link
Collaborator

Before proceeding with the implementation of new plugin-level CLI options (--onlyAudits, --skipAudits, etc.), we need to resolve an existing issue in the Lighthouse plugin where unexpected errors are thrown when using onlyAudits and onlyGroups directly in the plugin configuration.

The new issue tracking this bug is #903

Once this bug is resolved, we can safely implement and test the new CLI options.

@BioPhoton
Copy link
Collaborator Author

I thought about a wrapper that supports in creating plugins and handles a couple of things:

  • filter audits,groups,categories
  • handles global options like verbose
  • enabled hooks for caching

Wdyt?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug something isn't working 🧩 core 🤓 UX UX improvement for CLI users
Projects
None yet
Development

No branches or pull requests

3 participants