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

OrganizationsService.GetPackage escapes packagName, but UserService.GetPackage does not #3262

Open
jhrozek opened this issue Sep 11, 2024 · 6 comments
Assignees
Labels

Comments

@jhrozek
Copy link

jhrozek commented Sep 11, 2024

It seems that the OrganizationsService does escape package names by calling url.PathEscape, but the UserService does not.

Is this a bug or is there a reason behind escaping one of the APIs but not the other?

If this is a simple bug, I'll be happy to submit a fix.

@gmlewis
Copy link
Collaborator

gmlewis commented Sep 11, 2024

This looks like a bug to me. Sounds great... thank you, @jhrozek !
Please remember to add unit tests that demonstrate at least one package name that requires escaping.

@gmlewis
Copy link
Collaborator

gmlewis commented Sep 11, 2024

Also, our CONTRIBUTING.md guide has useful information in it.

@DiegoDev2
Copy link
Contributor

hi! @gmlewis I'm interested in the problem, apparently the boy didn't do anything about it.

@gmlewis
Copy link
Collaborator

gmlewis commented Nov 2, 2024

OK, thank you, @DiegoDev2 - it's yours.

@gmlewis gmlewis assigned DiegoDev2 and unassigned jhrozek Nov 2, 2024
@DiegoDev2
Copy link
Contributor

DiegoDev2 commented Nov 2, 2024

while I was testing some of the unit tests it gave me FAIL:

 go test -v orgs_packages_test.go
# command-line-arguments [command-line-arguments.test]
./orgs_packages_test.go:18:30: undefined: setup
./orgs_packages_test.go:22:3: undefined: testMethod
./orgs_packages_test.go:50:20: undefined: referenceTimeStr
./orgs_packages_test.go:51:20: undefined: referenceTimeStr
./orgs_packages_test.go:61:67: undefined: PackageListOptions
./orgs_packages_test.go:66:13: undefined: Package
./orgs_packages_test.go:67:17: undefined: Int64
./orgs_packages_test.go:68:17: undefined: String
./orgs_packages_test.go:69:17: undefined: String
./orgs_packages_test.go:70:17: undefined: Int64
./orgs_packages_test.go:70:17: too many errors
FAIL    command-line-arguments [build failed]
FAIL
go test -v users_packages_test.go
# command-line-arguments [command-line-arguments.test]
./users_packages_test.go:18:30: undefined: setup
./users_packages_test.go:22:3: undefined: testMethod
./users_packages_test.go:23:3: undefined: testFormValues
./users_packages_test.go:23:24: undefined: values
./users_packages_test.go:31:20: undefined: referenceTimeStr
./users_packages_test.go:32:20: undefined: referenceTimeStr
./users_packages_test.go:38:58: undefined: PackageListOptions
./users_packages_test.go:38:90: undefined: String
./users_packages_test.go:43:13: undefined: Package
./users_packages_test.go:44:17: undefined: Int64
./users_packages_test.go:44:17: too many errors
FAIL    command-line-arguments [build failed]
FAIL

does this not matter? Or should the test go well?

@gmlewis
Copy link
Collaborator

gmlewis commented Nov 2, 2024

@DiegoDev2 - That is not how you run unit tests in Go.

Try this instead: go test -v ./...

Please read CONTRIBUTING.md for more information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants