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.

Sign up for GitHub

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

Jump to bottom

x/website: module tutorial "add a test" should apply "keep going" principle" #68741

Open
matttproud opened this issue Aug 6, 2024 · 6 comments · May be fixed by golang/website#297
Open

x/website: module tutorial "add a test" should apply "keep going" principle" #68741

matttproud opened this issue Aug 6, 2024 · 6 comments · May be fixed by golang/website#297
Labels
Documentation Issues describing a change to documentation. NeedsFix The path to resolution is known, but the work has not been done. website
Milestone
website/backlog

Comments

@matttproud
Copy link
Contributor

matttproud commented Aug 6, 2024 •
edited
Loading

Go version

n/a

Output of go env in your module/workspace:

n/a

What did you do?

I visisted https://go.dev/doc/tutorial/add-a-test and I read how to create a test for Go code.

What did you see happen?

I saw it advise test construction using (*testing.T).Fatalf per the excerpts below:

func TestHelloName(t *testing.T) {
    name := "Gladys"
    want := regexp.MustCompile(`\b`+name+`\b`)
    msg, err := Hello("Gladys")
    if !want.MatchString(msg) || err != nil {
        t.Fatalf(`Hello("Gladys") = %q, %v, want match for %#q, nil`, msg, err, want)
    }
}

func TestHelloEmpty(t *testing.T) {
    msg, err := Hello("")
    if msg != "" || err == nil {
        t.Fatalf(`Hello("") = %q, %v, want "", error`, msg, err)
    }
}

What did you expect to see?

Given the design guidance with tests to "keep going", I would expect these tests to be written as below using (*testing.T).Errorf:

func TestHelloName(t *testing.T) {
    name := "Gladys"
    want := regexp.MustCompile(`\b`+name+`\b`)
    msg, err := Hello("Gladys")
    if !want.MatchString(msg) || err != nil {
        t.Errorf(`Hello("Gladys") = %q, %v, want match for %#q, nil`, msg, err, want)
    }
}

func TestHelloEmpty(t *testing.T) {
    msg, err := Hello("")
    if msg != "" || err == nil {
        t.Errorf(`Hello("") = %q, %v, want "", error`, msg, err)
    }
}

It seems like it would be useful to have the training material aligned with community development expectations.
@just-an-engineer
Copy link

Hey, I'll make a quick PR for this

@just-an-engineer just-an-engineer linked a pull request Aug 6, 2024 that will close this issue
Open
@dr2chase dr2chase added the NeedsFix The path to resolution is known, but the work has not been done. label Aug 6, 2024
@dr2chase
Copy link
Contributor

dr2chase commented Aug 6, 2024

Related CL (that needs to mention this bug): https://go.dev/cl/603335

@dr2chase
Copy link
Contributor

dr2chase commented Aug 6, 2024

cc @golang/tools-team

@seankhliao seankhliao changed the title go.dev: module tutorial "add a test" should apply "keep going" principle" x/website: module tutorial "add a test" should apply "keep going" principle" Aug 6, 2024
@gopherbot gopherbot added this to the Unreleased milestone Aug 6, 2024
@gabyhelp
Copy link

gabyhelp commented Aug 6, 2024

Related Issues and Documentation

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/603335 mentions this issue: _content/doc/tutorial: apply the "keep going" principle to "add a test" examples

@hyangah hyangah modified the milestones: Unreleased, website/backlog Aug 8, 2024
@hyangah hyangah added the Documentation Issues describing a change to documentation. label Aug 8, 2024
@matttproud
Copy link
Contributor Author

Anything blocking submission of the pending changelist at this point?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Documentation Issues describing a change to documentation. NeedsFix The path to resolution is known, but the work has not been done. website
Projects
None yet
Milestone
website/backlog
6 participants
@matttproud @dr2chase @hyangah @gopherbot @just-an-engineer @gabyhelp