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

chore: redo rules tests #603

Merged
merged 9 commits into from
Feb 20, 2023
Merged

chore: redo rules tests #603

merged 9 commits into from
Feb 20, 2023

Conversation

vjerci
Copy link
Contributor

@vjerci vjerci commented Feb 17, 2023

Description

This Pr optimises rules tests in a way that we no longer perform e2e test but do integration tests for them instead.

Trough profiling we've found that biggest chunk cpu time is spent on compiling all the rules.
I've optimised it in a way that we use worker to compile all the rules only once, then reuse that worker.
It isn't without it's downsides. Because we reuse single testing worker, and because we use viper for default config, we can no longer run tests in parallel.
Getting tests to run in parallel is limited to single test case which translates to single rule (it usually has multiple files which run in parallel)

  • We might in future do some extra work to have a worker per language or some other logical split to further improve performance

However I've manged to improve the total time it takes to run our rules test by 2x-3x and allow us to run tests with debugger


summary changes

There are cases where we detect 2 different rules in summary.
Eg. of those rules is ruby_weak_encryption and ruby_weak_encryption_with_data


stats

localmachine (macbook pro)
old_summary - 187.077s
new - 51.512s

github runner
old_summary - 332s
new - 183.678s


related

#588

Checklist

  • I've added test coverage that shows my fix or feature works as expected.
  • I've updated or added documentation if required.
  • I've included usage information in the description if CLI behavior was updated or added.
  • PR title follows Conventional Commits format

@vjerci vjerci marked this pull request as ready for review February 17, 2023 17:03
@gotbadger gotbadger mentioned this pull request Feb 17, 2023
4 tasks
Copy link
Contributor

@elsapet elsapet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙏

integration/rules/javascript_test.go Outdated Show resolved Hide resolved
@vjerci vjerci merged commit 528dd0b into main Feb 20, 2023
@vjerci vjerci deleted the chore/integration-tests branch February 20, 2023 11:19
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

Successfully merging this pull request may close these issues.

2 participants