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

Remove packages.NeedDeps #444

Merged
merged 3 commits into from Apr 5, 2022
Merged

Remove packages.NeedDeps #444

merged 3 commits into from Apr 5, 2022

Conversation

LandonTClipp
Copy link
Contributor

@LandonTClipp LandonTClipp commented Apr 5, 2022

This was introduced #435 but it wasn't the correct solution.

This was introduced #435 but it wasn't the correct solution.
@codecov-commenter
Copy link

codecov-commenter commented Apr 5, 2022

Codecov Report

Merging #444 (ed38b20) into master (8384e25) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #444   +/-   ##
=======================================
  Coverage   69.80%   69.80%           
=======================================
  Files           7        7           
  Lines        1262     1262           
=======================================
  Hits          881      881           
  Misses        333      333           
  Partials       48       48           
Impacted Files Coverage Δ
pkg/parse.go 81.43% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8384e25...ed38b20. Read the comment docs.

@LandonTClipp LandonTClipp merged commit 4703d1a into master Apr 5, 2022
@LandonTClipp LandonTClipp deleted the remove_need_deps branch April 5, 2022 02:45
@emmanuel099
Copy link
Contributor

Since ba1f213 mock generation fails again with Unexpected package creation during export data loading

@emmanuel099
Copy link
Contributor

emmanuel099 commented Apr 6, 2022

Tested it with all versions since 2.10.0:

  • v2.10.0:
    • Fails with "internal error: package "context" without types was imported from ..." (using official binary)
    • Fails with "Unexpected package creation during export data loading" (using go install with golang 1.18; this mistakenly fixed the "package without types" problem because the binary was build with 1.18 instead of 1.16)
  • v2.10.1: Works
  • v2.10.2: Works
  • v2.10.3: Fails with "Unexpected package creation during export data loading"
  • v2.10.4: Fails with "Unexpected package creation during export data loading"

@LandonTClipp
Copy link
Contributor Author

LandonTClipp commented Apr 7, 2022

Arg, I'm not sure why this isn't getting caught in the matrix test. Adding the packages.NeedDeps isn't an appropriate solution because it increases runtime by orders of magnitude. Can you see anything else we can do? First step would be to get a unit test to try to isolate this, then fix the problem some other way. I will look through your example to see if there is another way.

@LandonTClipp
Copy link
Contributor Author

LandonTClipp commented Apr 7, 2022

@emmanuel099 I am not able to replicate your issue on the latest build.

[lclipp@chilx-lcl1 testing-with-gomock][0] $ ls
doer  go.mod  LICENSE  match  mockery_2.10.4_Linux_x86_64.tar.gz  mocks  README.md  user
[lclipp@chilx-lcl1 testing-with-gomock][0] $ tar -xzf mockery_2.10.4_Linux_x86_64.tar.gz
[lclipp@chilx-lcl1 testing-with-gomock][0] $ ls
doer  go.mod  LICENSE  match  mockery  mockery_2.10.4_Linux_x86_64.tar.gz  mocks  README.md  user
[lclipp@chilx-lcl1 testing-with-gomock][0] $ ./mockery --version
07 Apr 22 18:49 CDT INF Starting mockery dry-run=false version=v2.10.4
v2.10.4
[lclipp@chilx-lcl1 testing-with-gomock][0] $ ./mockery --dir=doer --name=Doer
07 Apr 22 18:50 CDT INF Starting mockery dry-run=false version=v2.10.4
07 Apr 22 18:50 CDT INF Walking dry-run=false version=v2.10.4
07 Apr 22 18:50 CDT INF Generating mock dry-run=false interface=Doer qualified-name=github.com/sgreben/testing-with-gomock/doer version=v2.10.4
[lclipp@chilx-lcl1 testing-with-gomock][0] $ cat mocks/Doer.go
// Code generated by mockery v2.10.4. DO NOT EDIT.

package mocks

import mock "github.com/stretchr/testify/mock"

// Doer is an autogenerated mock type for the Doer type
type Doer struct {
        mock.Mock
}

// DoSomething provides a mock function with given fields: _a0, _a1
func (_m *Doer) DoSomething(_a0 int, _a1 string) error {
        ret := _m.Called(_a0, _a1)

        var r0 error
        if rf, ok := ret.Get(0).(func(int, string) error); ok {
                r0 = rf(_a0, _a1)
        } else {
                r0 = ret.Error(0)
        }

        return r0
}

Can you confirm your method of installation for mockery and which version of golang you are using?

Edit: ah, the demo repo is not a module (you can see I used go mod init in my reproducer attempt). Mockery requires you are using modules. I am open to ideas on allowing non-module repos but I don't want to devote too much effort into it, as the majority of people are using go modules.

@comphilip
Copy link

@LandonTClipp Unexpected package creation during export data loading is golang.org/x/tools issue

golang/go#51629

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.

None yet

4 participants