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

Test failures after forking with default settings #4921

Open
philrz opened this issue Dec 1, 2023 · 0 comments
Open

Test failures after forking with default settings #4921

philrz opened this issue Dec 1, 2023 · 0 comments
Labels
test Creating/improving test automation

Comments

@philrz
Copy link
Contributor

philrz commented Dec 1, 2023

tl;dr

A user that forks the Zed repo using GitHub defaults and then tries to run the tests will see these failures.

$ make test-system
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 29.2M    0 29.2M    0     0  10.7M      0 --:--:--  0:00:02 --:--:-- 10.7M
--- FAIL: TestZed (0.02s)
    --- FAIL: TestZed/cmd/zed/ztests (0.00s)
        --- FAIL: TestZed/cmd/zed/ztests/version (0.06s)
            ztest.go:389: cmd/zed/ztests/version.yaml: stdout: regexp "Version: v[0-9]+\\..*\n" does not match "Version: 245d765c\n"
    --- FAIL: TestZed/cmd/zq/ztests (0.01s)
        --- FAIL: TestZed/cmd/zq/ztests/version (0.05s)
            ztest.go:389: cmd/zq/ztests/version.yaml: stdout: regexp "Version: v[0-9]+\\..*\n" does not match "Version: 245d765c\n"
    --- FAIL: TestZed/service/ztests (0.04s)
        --- FAIL: TestZed/service/ztests/version (0.35s)
            ztest.go:389: service/ztests/version.yaml: stdout: regexp "v[0-9]*\\.[0-9]*.*\n" does not match "Version: 245d765c\n"
FAIL
FAIL	github.com/brimdata/zed	31.821s
FAIL
make: *** [test-system] Error 1

Details

Repro is with Zed commit 245d765.

While assisting a community user with test failures as part of #4907, I bumped into the failures above that were not their fault. The version strings were looking like:

$ zed -version
Version: 245d765c

While the tests expect them to look like:

$ zed -version
Version: v1.11.1-18-g245d765c

After a tip from @nwt, I've confirmed that the problem is triggered by going along with the default checkbox shown here in the GitHub workflow for creating the fork. By sticking with this default, no tags are made on the forked repo, and the way the version string is constructed happens to be reliant on tags.

image

I've confirmed that by actively unchecking the box when creating the fork, the version string gets constructed fine and all the tests pass.

I can imagine multiple ways to address.

  1. We could just add a note to CONTRIBUTING.md advising users to uncheck the box when creating their fork in order to avoid this problem. In my experience, though, many users miss these kinds of specifics in docs, so at best it might just make us feel more smug if they hit the problem and we can say "RTFM". 🤪

  2. We could relax the regex match in the failing tests to treat the shorter version strings as ok. Indeed, they still show the commit hash, so in spirit I'm personally fine with this. The times we've had bugs with the version strings in the past they came out empty, so as long as the tests still catch that, I'd be happy.

  3. Maybe other folks have ideas, such as constructing the version string in a different way that's not vulnerable to this problem.

@philrz philrz added the test Creating/improving test automation label Dec 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Creating/improving test automation
Projects
None yet
Development

No branches or pull requests

1 participant