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

Fix content tests; run tests via GitHub Actions #260

Merged
merged 16 commits into from
Jun 24, 2024
Merged

Fix content tests; run tests via GitHub Actions #260

merged 16 commits into from
Jun 24, 2024

Conversation

alexandear
Copy link
Contributor

@alexandear alexandear commented Jun 14, 2024

The PR fixes the test that checks code examples in the _content/tour directory and adds GitHub Actions workflow to run tests on each PR or push to the main branch.

Now, running go test -v ./... works flawlessly.

I have divided the PR into separate logical commits to facilitate review. Each commit corresponds to one logical change.

Changes:

  • Fixed TestContent to work with //go:build build constraint. It's worth mentioning that the tour from Go team's uses // +build comments, but this gotour uses //go:build (introduced here drop legacy build tag #45). Thus, TestContent has been adapted to work with //go:build.
  • Moved content_test.go to the root because go test skips the _content folder when detecting tests.
  • Renamed TestContent to TestContentTour.
  • Retained only one content_test.go that checks examples for all languages.
  • Added the missing OMIT to all code snippets.
  • Added the build constraint nobuild to code snippets that cannot be compiled via go build.
  • Added the build constraint norun to code snippets that cannot be executed. Note that we cannot use no-run (with a hyphen) as a build constraint name.

@ardan-bkennedy
Copy link
Contributor

We would need to review this together before I can accept it. I don't run any tests since the code was taken from the Go team and I don't update it. I just check the site and publish.

I'm not adverse but we will need to jump on a call.

@alexandear alexandear marked this pull request as ready for review June 15, 2024 14:18
@alexandear
Copy link
Contributor Author

@ardan-bkennedy I updated the PR's description with extended explanation. Hope this clarifies changes a bit.

@ardan-bkennedy
Copy link
Contributor

It's too big and I want to review this with you if I'm going to accept it. I'll ping you during the week

@ardan-bkennedy
Copy link
Contributor

Give me a PR to the bill/testing branch please.

@alexandear alexandear changed the base branch from main to bill/testing June 16, 2024 16:33
@alexandear
Copy link
Contributor Author

Give me a PR to the bill/testing branch please.

Changed base branch to the bill/testing.

@ardan-bkennedy ardan-bkennedy merged commit ebafc09 into ardanlabs:bill/testing Jun 24, 2024
@alexandear
Copy link
Contributor Author

Hello @ardan-bkennedy,
I noticed that you merged into the bill/testing branch. Is there a chance this PR will be merged into the main branch?

@ardan-bkennedy
Copy link
Contributor

I could not get the tests to work. When I run go test ./... everything hangs. I have not had the time to review why.

@alexandear
Copy link
Contributor Author

The tests are not hanging; they are just slow. I changed the test to target only the eng directory (see the PR #262), and it took 50 seconds to complete:

❯ go test ./...
?       github.com/ardanlabs/gotour/cmd/markdown-to-site        [no test files]
?       github.com/ardanlabs/gotour/cmd/tour    [no test files]
?       github.com/ardanlabs/gotour/internal/tour       [no test files]
?       github.com/ardanlabs/gotour/internal/socket     [no test files]
ok      github.com/ardanlabs/gotour     49.456s
ok      github.com/ardanlabs/gotour/internal/webtest    (cached)

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.

2 participants