assert: ensure message is always displayed & fix under bazel by cstrahan · Pull Request #276 · gotestyourself/gotest.tools

cstrahan · 2023-10-02T20:23:21Z

Note that I still need to run the tests, but I wanted to get this PR open ASAP.

This PR fixes #274.

Now, when running under bazel test we'll see the following sort of error message when the target test source file can't be found:

--- FAIL: TestAssertShouldWorkUnderBazel (0.00s)
    your_test.go:40: the test source file does not exist: /private/var/tmp/_bazel_charles/99cf13450b07a7c7594bbbe785fbd4ef/sandbox/darwin-sandbox/20/execroot/__main__/bazel-out/darwin_arm64-fastbuild/bin/some_package/your_package_test_/your_package_test.runfiles/__main__/some_package/your_test.go
        it appears that you are running this test under Bazel (target: //some_package:your_package_test);
        check that your test source files are added as test data in your go_test targets.
        example:
            go_test(
                name = "your_package_test",
                     srcs = ["your_test.go"],
                     deps = ["@tools_gotest_v3//assert"],
                     data = glob(["*_test.go"] # <====== test source files added as test data here!
            )
    your_test.go:40: assertion failed: but assert failed to find the expression to print: a should equal b
FAIL

If the user adds the test source file to their test data in the respective go_test target, they'll now see something like:

--- FAIL: TestAssertShouldWorkUnderBazel (0.00s)
    your_test.go:40: assertion failed: a is not b: a should equal b
FAIL

CLAassistant · 2023-10-02T20:23:28Z

All committers have signed the CLA.

cstrahan · 2023-10-02T20:25:51Z

All tests run via go test ./... pass, with the exception of these two (which I believe are due to an issue with the tests when run under macOS, as they were failing before my changes):

--- FAIL: TestChangeWorkingDir (0.00s)
    --- FAIL: TestChangeWorkingDir/changed_to_dir (0.00s)
        env_test.go:138: assertion failed: /private/var/folders/vc/8x3823591cz2_gpk94zpjfv00000gq/T/TestChangeWorkingDir-1487778779 (string) != /var/folders/vc/8x3823591cz2_gpk94zpjfv00000gq/T/TestChangeWorkingDir-1487778779 (string)
--- FAIL: TestChangeWorkingDir_IntegrationWithCleanup (0.00s)
    --- FAIL: TestChangeWorkingDir_IntegrationWithCleanup/cleanup_in_subtest (0.00s)
        env_test.go:157: assertion failed: /private/var/folders/vc/8x3823591cz2_gpk94zpjfv00000gq/T/TestChangeWorkingDir_IntegrationWithCleanup-2603325723 (string) != /var/folders/vc/8x3823591cz2_gpk94zpjfv00000gq/T/TestChangeWorkingDir_IntegrationWithCleanup-2603325723 (string)

I'll see if I can fix the lint issue.

cstrahan · 2023-10-02T20:45:11Z

Hmm. The ci/circleci: test-golang-1.20 check is failing:

∅  .
✓  assert (689ms)
∅  icmd/internal/stub
∅  internal/assert
∅  internal/cleanup
∅  internal/difflib
∅  internal/maint
∅  x
✓  internal/format (71ms)
✓  assert/opt (72ms)
✓  skip (75ms)
✓  env (75ms)
✓  assert/cmp (75ms)
✓  golden (75ms)
✓  internal/source (76ms)
✓  fs (170ms)
✖  poll (271ms)
✓  icmd (1.411s)
✓  x/subtest (4ms)
✓  assert/cmd/gty-migrate-from-testify (20.483s)

=== Skipped
=== SKIP: env TestPatchAllWindows (0.00s)
    env_test.go:70: runtime.GOOS != "windows"

=== Failed
=== FAIL: poll TestWaitOn_WithCompare (0.09s)
    poll_test.go:86: assertion failed: string "timeout hit after 10ms: first check never completed" does not contain "assertion failed: 3 (int) != 4 (int)"

DONE 289 tests, 1 skipped, 1 failure in 54.413s

Exited with code exit status 1

but that test passes with [email protected] on my machine (running macOS). Maybe a flaky test? Might bump CI with an empty commit.

cstrahan · 2023-10-02T20:51:54Z

Yay! The tests pass. But I just realized that my editor was hiding the fact that tabs and spaces got mixed together in that big error message string. Just pushed a commit to fix that up.

And move logic to a function so that variables can be closer to where they are used

dnephin

Thank you for the PR! I made a couple small changes, but overall this looks great.

Primarily I moved the variables to a bazel.go file, and extracted some of the logic to a new function. This way the variables and the code can be located a bit closer together.

I also extracted the multi-line error message to a raw string to make it a bit easier to read in code. The raw string removes the need to escape the double quotes.

If these changes look alright to you I think this is ready to be merged.

dnephin · 2023-10-09T21:13:09Z

internal/source/bazel.go

+        name = "your_package_test",
+        srcs = ["your_test.go"],
+        deps = ["@tools_gotest_v3//assert"],
+        data = glob(["*_test.go"])


On this line I added the trailing ) which I believe was missing, and removed the "test source files added as test data here!" comment. Does that seem ok? I guess the glob should capture all the test files.

Thanks! Yes, I think this is great 👍

cstrahan · 2023-10-11T16:27:28Z

@dnephin Those changes look great! Don't see anything else I'd change. Thanks!

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [gotest.tools/v3](https://github.com/gotestyourself/gotest.tools) | require | patch | `v3.5.1` -> `v3.5.2` | --- ### Release Notes <details> <summary>gotestyourself/gotest.tools (gotest.tools/v3)</summary> ### [`v3.5.2`](https://github.com/gotestyourself/gotest.tools/releases/tag/v3.5.2) [Compare Source](gotestyourself/gotest.tools@v3.5.1...v3.5.2) #### What's Changed - assert: ensure message is always displayed & fix under bazel by [@cstrahan](https://github.com/cstrahan) in gotestyourself/gotest.tools#276 - go.mod: golang.org/x/tools v0.13.0 for go1.22+ compatibility by [@thaJeztah](https://github.com/thaJeztah) in gotestyourself/gotest.tools#282 - poll: Continue(): use format.Message for formatting by [@thaJeztah](https://github.com/thaJeztah) in gotestyourself/gotest.tools#279 - fix TestFromDirSymlink on Windows due to missing drive-letter by [@thaJeztah](https://github.com/thaJeztah) in gotestyourself/gotest.tools#283 - Fix various linting issues and minor bugs by [@thaJeztah](https://github.com/thaJeztah) in gotestyourself/gotest.tools#280 - fix badges in readme, gofmt, and minor linting fix by [@thaJeztah](https://github.com/thaJeztah) in gotestyourself/gotest.tools#284 - circleci: add go1.21, go1.22, go1.23, and update golangci-lint to v1.60.3 by [@thaJeztah](https://github.com/thaJeztah) in gotestyourself/gotest.tools#285 - assert, assert/cmp: un-deprecate assert.ErrorType for now by [@thaJeztah](https://github.com/thaJeztah) in gotestyourself/gotest.tools#286 #### New Contributors - [@cstrahan](https://github.com/cstrahan) made their first contribution in gotestyourself/gotest.tools#276 **Full Changelog**: gotestyourself/gotest.tools@v3.5.1...v3.5.2 </details> --- ### Configuration 📅 **Schedule**: Branch creation - Between 12:00 AM and 03:59 AM ( * 0-3 * * * ) (UTC), Automerge - Between 12:00 AM and 03:59 AM ( * 0-3 * * * ) (UTC). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Reviewed-on: https://code.forgejo.org/forgejo/runner/pulls/524 Co-authored-by: Renovate Bot <[email protected]> Co-committed-by: Renovate Bot <[email protected]>

assert: ensure message is always displayed & fix under bazel

609c613

cstrahan added 2 commits October 2, 2023 15:26

fix lint

bfcc174

fix other lint

49646b6

cstrahan added 2 commits October 2, 2023 15:45

CI

33b7500

don't mix tabs and spaces in message

fbe56d2

cstrahan mentioned this pull request Oct 3, 2023

runtime.Caller gives different results under go test vs bazel test bazel-contrib/rules_go#3690

Closed

Improve bazel error message

91f77fd

And move logic to a function so that variables can be closer to where they are used

dnephin approved these changes Oct 9, 2023

View reviewed changes

dnephin merged commit 1fa4ef3 into gotestyourself:main Oct 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

assert: ensure message is always displayed & fix under bazel#276

assert: ensure message is always displayed & fix under bazel#276
dnephin merged 6 commits intogotestyourself:mainfrom
cstrahan:bazel-fix

cstrahan commented Oct 2, 2023 •

edited

Loading

Uh oh!

CLAassistant commented Oct 2, 2023 •

edited

Loading

Uh oh!

cstrahan commented Oct 2, 2023

Uh oh!

cstrahan commented Oct 2, 2023

Uh oh!

cstrahan commented Oct 2, 2023

Uh oh!

dnephin left a comment

Uh oh!

dnephin Oct 9, 2023

Uh oh!

cstrahan Oct 11, 2023

Uh oh!

cstrahan commented Oct 11, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

cstrahan commented Oct 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CLAassistant commented Oct 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cstrahan commented Oct 2, 2023

Uh oh!

cstrahan commented Oct 2, 2023

Uh oh!

cstrahan commented Oct 2, 2023

Uh oh!

dnephin left a comment

Choose a reason for hiding this comment

Uh oh!

dnephin Oct 9, 2023

Choose a reason for hiding this comment

Uh oh!

cstrahan Oct 11, 2023

Choose a reason for hiding this comment

Uh oh!

cstrahan commented Oct 11, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cstrahan commented Oct 2, 2023 •

edited

Loading

CLAassistant commented Oct 2, 2023 •

edited

Loading