Skip to content

Test/GitHub oauth#76

Open
Fatimasanusi wants to merge 3 commits into
Grainlify:mainfrom
Fatimasanusi:test/github-oauth
Open

Test/GitHub oauth#76
Fatimasanusi wants to merge 3 commits into
Grainlify:mainfrom
Fatimasanusi:test/github-oauth

Conversation

@Fatimasanusi

Copy link
Copy Markdown
Contributor

closes #39

@Jagadeeshftw

Copy link
Copy Markdown
Contributor

thanks for adding oauth test coverage, the table-driven tests for AuthorizeURL, joinScopes and ExchangeCode (including timeout and context cancellation) look thorough and are exactly what this package needs.

i held off merging because internal/github/oauth.go in this branch does not compile. go build reports:

internal/github/oauth.go:22:1: syntax error: non-declaration statement outside function body
internal/github/oauth.go:39:7: syntax error: unexpected name out at end of statement
internal/github/oauth.go:52:6: syntax error: unexpected name ExchangeCode, expected (

looks like a few edits got mangled in oauth.go specifically:

  • the func AuthorizeURL(...) header is gone, line 22 starts with a stray 'tq := u.Query()' that is outside any function (and tq is undefined)
  • joinScopes is truncated mid-loop: 'out += " "' is followed by 'eturn out', so the inner '}', 'out += s', the loop close and most of 'return' are missing
  • in ExchangeCode the config-check if loses its closing brace and a doc comment block gets inserted into the function body, with a stray '}' tacked onto the end of a comment line
  • the request also calls a tokenEndpoint identifier that is not declared anywhere (was previously the literal token URL)

the test file itself is fine. could you restore oauth.go to a buildable state (the AuthorizeURL/joinScopes/ExchangeCode bodies as they were, and either declare tokenEndpoint or keep the literal url) and push again? happy to merge as soon as go build ./... and go test ./internal/github/... pass. thanks!

@Fatimasanusi

Copy link
Copy Markdown
Contributor Author

Okay I'll work on that

@Jagadeeshftw

Copy link
Copy Markdown
Contributor

thanks for this. heads up before we can merge: internal/github/oauth.go still does not compile. running go build ./internal/github/... fails with syntax errors, e.g. 'tq := u.Query()' sitting outside any function body around line 22, a truncated joinScopes that ends with 'eturn out' on line 39, and a missing func declaration for the authorize-url builder plus a broken ExchangeCode body near lines 54-60. looks like the authorize-url helper got partially deleted/garbled. could you fix oauth.go so it builds cleanly and i'll merge. leaving this open for now.

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.

Add unit tests for internal/github OAuth helpers (AuthorizeURL, joinScopes, ExchangeCode)

2 participants