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

Add packages config: increases mock generation speed (5x) #548

Merged
merged 9 commits into from
Mar 5, 2023
Merged

Add packages config: increases mock generation speed (5x) #548

merged 9 commits into from
Mar 5, 2023

Conversation

LandonTClipp
Copy link
Collaborator

@LandonTClipp LandonTClipp commented Feb 11, 2023

Description

Mock generation speed is dramatically increased by specifying the packages to load in config instead of dynamically finding them.

The issue with the previous logic is that a package.Load would be called once per file, even if multiple files were part of the same package. We can dramatically improve performance by giving a config file that states which packages should be generated.

#520

In this PR:

[ltclipp@landon-virtualbox mockery]$ time ./mockery |& tail
real    0m0.707s
user    0m0.183s
sys     0m0.126s

Previously:

[ltclipp@landon-virtualbox mockery]$ time ./mockery |& tail
real    0m3.583s
user    0m2.710s
sys     0m1.329s

Deprecations

When packages is defined:

(ve) ➜  mockery git:(i_am_speed) ✗ ./mockery --log-level warn   
11 Feb 23 19:28 CST WRN ALPHA FEATURE: use of the 'packages' config variable is currently in an alpha state. Use at your own risk. discussion=https://github.com/vektra/mockery/discussions/549 dry-run=false version=v0.0.0-dev

When packages is not defined:

(ve) ➜  mockery git:(i_am_speed) ✗ ./mockery --log-level warn
11 Feb 23 19:29 CST INFO DISCUSSION: dynamic walking of project is being considered for removal in v3. Please provide your feedback at the linked discussion. discussion=https://github.com/vektra/mockery/discussions/549 dry-run=false pr=https://github.com/vektra/mockery/pull/548 version=v0.0.0-dev

Configuration

You can see we have a configuration that looks like this for the mockery project:

packages:
  github.com/vektra/mockery/v2/pkg:
    interfaces:
      TypesPackage:
  github.com/vektra/mockery/v2/pkg/fixtures:
    interfaces:
      RequesterArgSameAsNamedImport:
      RequesterVariadic:
        config:
          with-expecter: False
        configs:
          - structname: RequesterVariadicOneArgument
            unroll-variadic: False
          - structname: RequesterVariadic
      Expecter:
      RequesterReturnElided:

This is extremely powerful as we can load in any Golang package we want, and we have fine-grained control over where it gets installed. The defaults are to install into the mocks directory with a sub-path identical to the package's path (basically, we keep the same namespace as the go ecosystem).

Screenshot 2023-02-22 at 12 06 15 AM

TODO

  • Add tests
  • Have config keys be a map, not a list of strings (this will support future configurability)
  • Fix yaml parsing of nested packages section not including empty dicts. We want to "enable" an interface by simply specifying an empty dictionary.
  • Documentation
  • User comments

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Version of Golang used when building/testing:

  • 1.11
  • 1.12
  • 1.13
  • 1.14
  • 1.15
  • 1.16
  • 1.17
  • 1.18
  • 1.19
  • 1.20

How Has This Been Tested?

Ran mock generation manually and compared runtime to old method.

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@LandonTClipp LandonTClipp changed the title Dramatically increase mock generation speed Dramatically increase mock generation speed (5x performance increase) Feb 11, 2023
@codecov
Copy link

codecov bot commented Feb 12, 2023

Codecov Report

Patch coverage: 61.63982% and project coverage change: +0.68781 🎉

Comparison is base (02edbfe) 72.59201% compared to head (82bc0c1) 73.27982%.

Additional details and impacted files
@@                 Coverage Diff                 @@
##              master        #548         +/-   ##
===================================================
+ Coverage   72.59201%   73.27982%   +0.68780%     
===================================================
  Files              6           7          +1     
  Lines           1277        1744        +467     
===================================================
+ Hits             927        1278        +351     
- Misses           306         382         +76     
- Partials          44          84         +40     
Impacted Files Coverage Δ
cmd/showconfig.go 29.16667% <0.00000%> (ø)
pkg/outputter.go 34.87179% <34.35115%> (-0.51284%) ⬇️
pkg/config/config.go 60.00000% <60.00000%> (ø)
cmd/mockery.go 63.55140% <64.18605%> (+34.97997%) ⬆️
pkg/generator.go 93.29529% <81.25000%> (-0.90115%) ⬇️
pkg/parse.go 78.81773% <85.07463%> (-2.61940%) ⬇️
pkg/walker.go 64.54545% <93.33333%> (+8.77622%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@LandonTClipp
Copy link
Collaborator Author

LandonTClipp commented Feb 12, 2023

TODO

@LandonTClipp LandonTClipp changed the title Dramatically increase mock generation speed (5x performance increase) Add packages config: increases mock generation speed (5x) Feb 21, 2023
Mock generation speed is dramatically increased by specifying the
packages to load in config instead of dynamically finding them.

The issue with the previous logic is that a `package.Load` would be
called once per file, even if multiple files were part of the same
package. We can dramatically improve performance by giving a config file
that states which packages should be generated.

Add `with-expecter: True`

Logic fixes
Restructuring config so that each object has its own set of config. This
makes it easier to reason about which parts of the code are using which
pieces of the config.

Add config methods for parsing packages/interfaces

Methods have been added that allows parsing of the complex
`packages` config structure. Also adding large number of tests
to assert it behaves as expected.

Reshuffle the location of some code

More additions:

currently, mocks are failing to generate. All of the mocks are being given the
name `foo`, and I'm not sure why. Next on the agenda is to figure that piece
out and make the mocks have the correct struct names.

Allow config to specify `configs` section

This will let us specify multiple different mocks from
the same interface.

Work around issue with viper map parsing

Viper throws away config with empty maps. Our config allows empty maps
so we need additional logic to manually parse the `packages` section.

add more tests
@LandonTClipp LandonTClipp marked this pull request as ready for review March 4, 2023 06:01
@LandonTClipp LandonTClipp merged commit 16546bb into vektra:master Mar 5, 2023
@LandonTClipp LandonTClipp deleted the i_am_speed branch March 5, 2023 04:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant