mock: simplify implementation of FunctionalOptions #1571

dolmen · 2024-03-07T13:28:28Z

Summary

Remove unnecessary use of reflect in the implementation of mock.FunctionalOptions().

Changes

Change the internal representation of FunctionalOptionsArgument to be a slice of arguments to match the variadic argument of FunctionalOptions.
Remove unnecessary use of reflect.

Motivation

Simpler code
Runtime speed

dolmen · 2024-03-22T13:05:08Z

@snirye Could you review this change as you are a mock.FunctionalOptions user?

dolmen · 2024-03-22T13:10:14Z

@nbaztec Could you review this change as you wrote the initial implementation?

brackendawson

LGTM

I did some testing. No behavioural change but less uncecessary reflection.

Remove unnecessary use of reflect in the implementation of mock.FunctionalOptions().

dolmen · 2024-04-23T13:18:15Z

Rebased (with conflict resolution) after merge of f1b5324 (#1433).

brackendawson

Still LGTM

andrewwillette · 2024-06-08T18:51:03Z

@dolmen can this be merged? I'm hoping to unblock 1578.

…nercloud/fleeting-plugin-hetzner!175) This MR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [github.com/stretchr/testify](https://github.com/stretchr/testify) | require | minor | `v1.9.0` -> `v1.10.0` | --- ### Release Notes <details> <summary>stretchr/testify (github.com/stretchr/testify)</summary> ### [`v1.10.0`](https://github.com/stretchr/testify/releases/tag/v1.10.0) [Compare Source](stretchr/testify@v1.9.0...v1.10.0) #### What's Changed ##### Functional Changes - Add PanicAssertionFunc by [@fahimbagar](https://github.com/fahimbagar) in stretchr/testify#1337 - assert: deprecate CompareType by [@dolmen](https://github.com/dolmen) in stretchr/testify#1566 - assert: make YAML dependency pluggable via build tags by [@dolmen](https://github.com/dolmen) in stretchr/testify#1579 - assert: new assertion NotElementsMatch by [@hendrywiranto](https://github.com/hendrywiranto) in stretchr/testify#1600 - mock: in order mock calls by [@ReyOrtiz](https://github.com/ReyOrtiz) in stretchr/testify#1637 - Add assertion for NotErrorAs by [@palsivertsen](https://github.com/palsivertsen) in stretchr/testify#1129 - Record Return Arguments of a Call by [@jayd3e](https://github.com/jayd3e) in stretchr/testify#1636 - assert.EqualExportedValues: accepts everything by [@redachl](https://github.com/redachl) in stretchr/testify#1586 ##### Fixes - assert: make tHelper a type alias by [@dolmen](https://github.com/dolmen) in stretchr/testify#1562 - Do not get argument again unnecessarily in Arguments.Error() by [@TomWright](https://github.com/TomWright) in stretchr/testify#820 - Fix time.Time compare by [@myxo](https://github.com/myxo) in stretchr/testify#1582 - assert.Regexp: handle \[]byte array properly by [@kevinburkesegment](https://github.com/kevinburkesegment) in stretchr/testify#1587 - assert: collect.FailNow() should not panic by [@marshall-lee](https://github.com/marshall-lee) in stretchr/testify#1481 - mock: simplify implementation of FunctionalOptions by [@dolmen](https://github.com/dolmen) in stretchr/testify#1571 - mock: caller information for unexpected method call by [@spirin](https://github.com/spirin) in stretchr/testify#1644 - suite: fix test failures by [@stevenh](https://github.com/stevenh) in stretchr/testify#1421 - Fix issue [#1662](stretchr/testify#1662) (comparing infs should fail) by [@ybrustin](https://github.com/ybrustin) in stretchr/testify#1663 - NotSame should fail if args are not pointers [#1661](stretchr/testify#1661) by [@sikehish](https://github.com/sikehish) in stretchr/testify#1664 - Increase timeouts in Test_Mock_Called_blocks to reduce flakiness in CI by [@sikehish](https://github.com/sikehish) in stretchr/testify#1667 - fix: compare functional option names for indirect calls by [@arjun-1](https://github.com/arjun-1) in stretchr/testify#1626 ##### Documantation, Build & CI - .gitignore: ignore "go test -c" binaries by [@dolmen](https://github.com/dolmen) in stretchr/testify#1565 - mock: improve doc by [@dolmen](https://github.com/dolmen) in stretchr/testify#1570 - mock: fix FunctionalOptions docs by [@snirye](https://github.com/snirye) in stretchr/testify#1433 - README: link out to the excellent testifylint by [@brackendawson](https://github.com/brackendawson) in stretchr/testify#1568 - assert: fix typo in comment by [@JohnEndson](https://github.com/JohnEndson) in stretchr/testify#1580 - Correct the EventuallyWithT and EventuallyWithTf example by [@JonCrowther](https://github.com/JonCrowther) in stretchr/testify#1588 - CI: bump softprops/action-gh-release from 1 to 2 by [@dependabot](https://github.com/dependabot) in stretchr/testify#1575 - mock: document more alternatives to deprecated AnythingOfTypeArgument by [@dolmen](https://github.com/dolmen) in stretchr/testify#1569 - assert: Correctly document EqualValues behavior by [@brackendawson](https://github.com/brackendawson) in stretchr/testify#1593 - fix: grammar in godoc by [@miparnisari](https://github.com/miparnisari) in stretchr/testify#1607 - .github/workflows: Run tests for Go 1.22 by [@HaraldNordgren](https://github.com/HaraldNordgren) in stretchr/testify#1629 - Document suite's lack of support for t.Parallel by [@brackendawson](https://github.com/brackendawson) in stretchr/testify#1645 - assert: fix typos in comments by [@alexandear](https://github.com/alexandear) in stretchr/testify#1650 - mock: fix doc comment for NotBefore by [@alexandear](https://github.com/alexandear) in stretchr/testify#1651 - Generate better comments for require package by [@Neokil](https://github.com/Neokil) in stretchr/testify#1610 - README: replace Testify V2 notice with [@dolmen](https://github.com/dolmen)'s V2 manifesto by [@hendrywiranto](https://github.com/hendrywiranto) in stretchr/testify#1518 #### New Contributors - [@fahimbagar](https://github.com/fahimbagar) made their first contribution in stretchr/testify#1337 - [@TomWright](https://github.com/TomWright) made their first contribution in stretchr/testify#820 - [@snirye](https://github.com/snirye) made their first contribution in stretchr/testify#1433 - [@myxo](https://github.com/myxo) made their first contribution in stretchr/testify#1582 - [@JohnEndson](https://github.com/JohnEndson) made their first contribution in stretchr/testify#1580 - [@JonCrowther](https://github.com/JonCrowther) made their first contribution in stretchr/testify#1588 - [@miparnisari](https://github.com/miparnisari) made their first contribution in stretchr/testify#1607 - [@marshall-lee](https://github.com/marshall-lee) made their first contribution in stretchr/testify#1481 - [@spirin](https://github.com/spirin) made their first contribution in stretchr/testify#1644 - [@ReyOrtiz](https://github.com/ReyOrtiz) made their first contribution in stretchr/testify#1637 - [@stevenh](https://github.com/stevenh) made their first contribution in stretchr/testify#1421 - [@jayd3e](https://github.com/jayd3e) made their first contribution in stretchr/testify#1636 - [@Neokil](https://github.com/Neokil) made their first contribution in stretchr/testify#1610 - [@redachl](https://github.com/redachl) made their first contribution in stretchr/testify#1586 - [@ybrustin](https://github.com/ybrustin) made their first contribution in stretchr/testify#1663 - [@sikehish](https://github.com/sikehish) made their first contribution in stretchr/testify#1664 - [@arjun-1](https://github.com/arjun-1) made their first contribution in stretchr/testify#1626 **Full Changelog**: stretchr/testify@v1.9.0...v1.10.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

This MR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [github.com/stretchr/testify](https://github.com/stretchr/testify) | require | minor | `v1.9.0` -> `v1.10.0` | --- ### Release Notes <details> <summary>stretchr/testify (github.com/stretchr/testify)</summary> ### [`v1.10.0`](https://github.com/stretchr/testify/releases/tag/v1.10.0) [Compare Source](stretchr/testify@v1.9.0...v1.10.0) #### What's Changed ##### Functional Changes - Add PanicAssertionFunc by [@fahimbagar](https://github.com/fahimbagar) in stretchr/testify#1337 - assert: deprecate CompareType by [@dolmen](https://github.com/dolmen) in stretchr/testify#1566 - assert: make YAML dependency pluggable via build tags by [@dolmen](https://github.com/dolmen) in stretchr/testify#1579 - assert: new assertion NotElementsMatch by [@hendrywiranto](https://github.com/hendrywiranto) in stretchr/testify#1600 - mock: in order mock calls by [@ReyOrtiz](https://github.com/ReyOrtiz) in stretchr/testify#1637 - Add assertion for NotErrorAs by [@palsivertsen](https://github.com/palsivertsen) in stretchr/testify#1129 - Record Return Arguments of a Call by [@jayd3e](https://github.com/jayd3e) in stretchr/testify#1636 - assert.EqualExportedValues: accepts everything by [@redachl](https://github.com/redachl) in stretchr/testify#1586 ##### Fixes - assert: make tHelper a type alias by [@dolmen](https://github.com/dolmen) in stretchr/testify#1562 - Do not get argument again unnecessarily in Arguments.Error() by [@TomWright](https://github.com/TomWright) in stretchr/testify#820 - Fix time.Time compare by [@myxo](https://github.com/myxo) in stretchr/testify#1582 - assert.Regexp: handle \[]byte array properly by [@kevinburkesegment](https://github.com/kevinburkesegment) in stretchr/testify#1587 - assert: collect.FailNow() should not panic by [@marshall-lee](https://github.com/marshall-lee) in stretchr/testify#1481 - mock: simplify implementation of FunctionalOptions by [@dolmen](https://github.com/dolmen) in stretchr/testify#1571 - mock: caller information for unexpected method call by [@spirin](https://github.com/spirin) in stretchr/testify#1644 - suite: fix test failures by [@stevenh](https://github.com/stevenh) in stretchr/testify#1421 - Fix issue [#1662](stretchr/testify#1662) (comparing infs should fail) by [@ybrustin](https://github.com/ybrustin) in stretchr/testify#1663 - NotSame should fail if args are not pointers [#1661](stretchr/testify#1661) by [@sikehish](https://github.com/sikehish) in stretchr/testify#1664 - Increase timeouts in Test_Mock_Called_blocks to reduce flakiness in CI by [@sikehish](https://github.com/sikehish) in stretchr/testify#1667 - fix: compare functional option names for indirect calls by [@arjun-1](https://github.com/arjun-1) in stretchr/testify#1626 ##### Documantation, Build & CI - .gitignore: ignore "go test -c" binaries by [@dolmen](https://github.com/dolmen) in stretchr/testify#1565 - mock: improve doc by [@dolmen](https://github.com/dolmen) in stretchr/testify#1570 - mock: fix FunctionalOptions docs by [@snirye](https://github.com/snirye) in stretchr/testify#1433 - README: link out to the excellent testifylint by [@brackendawson](https://github.com/brackendawson) in stretchr/testify#1568 - assert: fix typo in comment by [@JohnEndson](https://github.com/JohnEndson) in stretchr/testify#1580 - Correct the EventuallyWithT and EventuallyWithTf example by [@JonCrowther](https://github.com/JonCrowther) in stretchr/testify#1588 - CI: bump softprops/action-gh-release from 1 to 2 by [@dependabot](https://github.com/dependabot) in stretchr/testify#1575 - mock: document more alternatives to deprecated AnythingOfTypeArgument by [@dolmen](https://github.com/dolmen) in stretchr/testify#1569 - assert: Correctly document EqualValues behavior by [@brackendawson](https://github.com/brackendawson) in stretchr/testify#1593 - fix: grammar in godoc by [@miparnisari](https://github.com/miparnisari) in stretchr/testify#1607 - .github/workflows: Run tests for Go 1.22 by [@HaraldNordgren](https://github.com/HaraldNordgren) in stretchr/testify#1629 - Document suite's lack of support for t.Parallel by [@brackendawson](https://github.com/brackendawson) in stretchr/testify#1645 - assert: fix typos in comments by [@alexandear](https://github.com/alexandear) in stretchr/testify#1650 - mock: fix doc comment for NotBefore by [@alexandear](https://github.com/alexandear) in stretchr/testify#1651 - Generate better comments for require package by [@Neokil](https://github.com/Neokil) in stretchr/testify#1610 - README: replace Testify V2 notice with [@dolmen](https://github.com/dolmen)'s V2 manifesto by [@hendrywiranto](https://github.com/hendrywiranto) in stretchr/testify#1518 #### New Contributors - [@fahimbagar](https://github.com/fahimbagar) made their first contribution in stretchr/testify#1337 - [@TomWright](https://github.com/TomWright) made their first contribution in stretchr/testify#820 - [@snirye](https://github.com/snirye) made their first contribution in stretchr/testify#1433 - [@myxo](https://github.com/myxo) made their first contribution in stretchr/testify#1582 - [@JohnEndson](https://github.com/JohnEndson) made their first contribution in stretchr/testify#1580 - [@JonCrowther](https://github.com/JonCrowther) made their first contribution in stretchr/testify#1588 - [@miparnisari](https://github.com/miparnisari) made their first contribution in stretchr/testify#1607 - [@marshall-lee](https://github.com/marshall-lee) made their first contribution in stretchr/testify#1481 - [@spirin](https://github.com/spirin) made their first contribution in stretchr/testify#1644 - [@ReyOrtiz](https://github.com/ReyOrtiz) made their first contribution in stretchr/testify#1637 - [@stevenh](https://github.com/stevenh) made their first contribution in stretchr/testify#1421 - [@jayd3e](https://github.com/jayd3e) made their first contribution in stretchr/testify#1636 - [@Neokil](https://github.com/Neokil) made their first contribution in stretchr/testify#1610 - [@redachl](https://github.com/redachl) made their first contribution in stretchr/testify#1586 - [@ybrustin](https://github.com/ybrustin) made their first contribution in stretchr/testify#1663 - [@sikehish](https://github.com/sikehish) made their first contribution in stretchr/testify#1664 - [@arjun-1](https://github.com/arjun-1) made their first contribution in stretchr/testify#1626 **Full Changelog**: stretchr/testify@v1.9.0...v1.10.0 </details> --- - [ ] If you want to rebase/retry this MR, check this box ---    See merge request alpine/infra/build-server-status!18

dolmen added pkg-mock Any issues related to Mock internal/refactor Refactor internals with no external visible changes mock.ArgumentMatcher About matching arguments in mock labels Mar 7, 2024

dolmen requested a review from brackendawson March 7, 2024 13:28

dolmen self-assigned this Mar 7, 2024

dolmen requested review from MovieStoreGuy and arjunmahishi March 15, 2024 11:28

dolmen mentioned this pull request Apr 1, 2024

mock: cleanup argument matching API #1578

Open

brackendawson previously approved these changes Apr 1, 2024

View reviewed changes

andrewwillette approved these changes Apr 1, 2024

View reviewed changes

mock: simplify implementation of FunctionalOptions

d4a63f5

Remove unnecessary use of reflect in the implementation of mock.FunctionalOptions().

dolmen dismissed brackendawson’s stale review via d4a63f5 April 23, 2024 13:11

dolmen force-pushed the mock-simplify-FunctionalOptions-impl branch from 2a14d5a to d4a63f5 Compare April 23, 2024 13:11

dolmen requested review from andrewwillette and brackendawson April 23, 2024 13:18

brackendawson approved these changes Apr 23, 2024

View reviewed changes

andrewwillette approved these changes Apr 28, 2024

View reviewed changes

Merge branch 'master' into mock-simplify-FunctionalOptions-impl

72e3b61

brackendawson merged commit 428847e into master Sep 30, 2024
14 checks passed

brackendawson deleted the mock-simplify-FunctionalOptions-impl branch September 30, 2024 22:30

felix-renovate bot mentioned this pull request Sep 29, 2025

fix(deps): update all non-major dependencies planetscale/terraform-provider-planetscale#121

Open

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

mock: simplify implementation of FunctionalOptions #1571

mock: simplify implementation of FunctionalOptions #1571

Uh oh!

dolmen commented Mar 7, 2024

dolmen commented Mar 22, 2024

dolmen commented Mar 22, 2024

brackendawson left a comment

dolmen commented Apr 23, 2024

brackendawson left a comment

andrewwillette commented Jun 8, 2024

Uh oh!

Labels

4 participants

mock: simplify implementation of FunctionalOptions #1571

mock: simplify implementation of FunctionalOptions #1571

Uh oh!

Conversation

dolmen commented Mar 7, 2024

Summary

Changes

Motivation

dolmen commented Mar 22, 2024

dolmen commented Mar 22, 2024

brackendawson left a comment

Choose a reason for hiding this comment

dolmen commented Apr 23, 2024

brackendawson left a comment

Choose a reason for hiding this comment

andrewwillette commented Jun 8, 2024

Uh oh!

Labels

4 participants