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

can not correctly mock GetReposContentsByOwnerByRepoByPath when path has "/" #26

Closed
LeoQuote opened this issue Mar 24, 2022 · 9 comments
Labels
bug Something isn't working

Comments

@LeoQuote
Copy link

		mock.WithRequestMatch(
			mock.GetReposContentsByOwnerByRepoByPath,
			github.RepositoryContent{
				Encoding: github.String("base64"),
				Path:     github.String("path/test-file.txt"),
				Content:  github.String("fake-content"),
			},
		),

if I call it with

client.Repositories.GetContents(
			*r.context, r.org, r.repo, github.String("path/test-file.txt"), &github.RepositoryContentGetOptions{})

it will raise exception

%!s(PANIC=Error method: runtime error: invalid memory address or nil pointer dereference)\nresponse: {{\"Response\":null,\"message\":\"mock response not found for /repos/owner/repo-name/contents/path/test-file.txt\",\"errors\":null}}"

seems to be the path could not match pattern

@migueleliasweb
Copy link
Owner

Oh boi, this seems to be an interesting edge case.

The {path} can actually contain forward slashes (I was unaware of that, tbh) and that seems to break the URL parsing for this endpoint. Without / in the path, the matching works.

@LeoQuote
Copy link
Author

hmm, I should urlencode this {path} or this is regarded as bug ?

@migueleliasweb
Copy link
Owner

I think I have a plan but that might require some extra smarts during the generation of the endpoints pattern in order to account for edge cases ( I'm sure there will be more eventually )

I can probably get this working soon-ish.

I will mark this as a bug.

@migueleliasweb
Copy link
Owner

Hi @LeoQuote, can you test the branch from #27.

Let me know if it fixes your problem.

@LeoQuote
Copy link
Author

Thanks for your quick response, will try it next week!

@LeoQuote
Copy link
Author

hmmm maybe releavant , I tried to mock PatchReposGitRefsByOwnerByRepoByRef also failed .

	mockedHTTPClient := mock.NewMockedHTTPClient(
		mock.WithRequestMatch(
			mock.GetReposByOwnerByRepo,
			github.Repository{
				DefaultBranch: github.String("base"),
				Name:          github.String("repo-name"),
			}),
		mock.WithRequestMatch(
			mock.PatchReposGitRefsByOwnerByRepoByRef,
			github.Reference{
				Ref: github.String("refs/heads/new-branch"),
			}),
	)
	c := github.NewClient(mockedHTTPClient)

	ctx := context.Background()
	_, _, err := c.Git.UpdateRef(ctx, "owner", "repo-name", &github.Reference{
		Ref:    github.String("refs/heads/new-branch"),
		Object: &github.GitObject{SHA: github.String("fake-sha")},
	}, false)

	assert.Nil(t, err)
     github_test.go:101: 
        	Error Trace:	github_test.go:101
        	Error:      	Expected nil, but got: &github.ErrorResponse{Response:(*http.Response)(nil), Message:"mock response not found for /repos/owner/repo-name/git/refs/heads/new-branch", Errors:[]github.Error(nil), Block:(*github.ErrorBlock)(nil), DocumentationURL:""}
        	Test:       	TestPush

@migueleliasweb
Copy link
Owner

@LeoQuote yeah, it's similar problem to the GetContents call. I will also fix that and create a new release.

@migueleliasweb
Copy link
Owner

@LeoQuote Can you test again your cases with #30? It should cater for both cases.

@migueleliasweb
Copy link
Owner

Merged #30

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants