-
Notifications
You must be signed in to change notification settings - Fork 86
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
add tests for serve methods #73
Conversation
a41c8d5
to
a9124b7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple of comments. Not totally in love with currentUser
as the name for the pkg level func, but I don't have a better idea.
{ | ||
name: "user link", | ||
link: "/me", | ||
wantStatus: http.StatusFound, | ||
wantLink: "/who/[email protected]", | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to add any negative tests here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
@@ -85,8 +235,8 @@ func TestExpandLink(t *testing.T) { | |||
for _, tt := range tests { | |||
t.Run(tt.name, func(t *testing.T) { | |||
got, err := expandLink(tt.long, expandEnv{Now: tt.now, Path: tt.remainder, User: tt.user}) | |||
if err != nil { | |||
t.Fatalf("expandLink(%q): %v", tt.long, err) | |||
if (err != nil) != tt.wantErr { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😵💫
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know... I'ver never found a way to test like this that I like. I've sometimes broken it apart into two different checks for whether an error is expected. 🤷🏻
Also check error returned from tmpl.Execute. Refactor currentUser to make the logic a little simpler, and make it a package var for easier testing. Signed-off-by: Will Norris <[email protected]>
a9124b7
to
aeae9f0
Compare
Also check error returned from tmpl.Execute. Refactor currentUser to make the logic a little simpler, and make it a package var for easier testing.
This doesn't make any behavioral changes (aside from checking the tmpl.Execute error). Instead, it's focused on making the next pass at #72 much simpler to review.