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

Allow "nonexistent version" test to work without brew #2432

Merged
merged 1 commit into from
Jan 21, 2025

Conversation

piperswe
Copy link
Contributor

Currently, this test fails if Homebrew isn't present as it expects a
Homebrew-specific output. This commit varies the expected output based
on whether Homebrew is installed.

This patch was originally created for the Debian package for ruby-build:
https://salsa.debian.org/ruby-team/ruby-build/-/blob/607093e5198cc75ffa8e9a3b551843be5f2f6d93/debian/patches/0001-Fix-nonexistent-version-test-to-work-regardless-of-e.patch

Copy link
Member

@mislav mislav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thank you for offering to make the test more stable. I'm unclear about the original failure on systems without brew, though. The test scenario in question has this line:

stub_repeated brew false

This creates a test-specific stub for the brew binary for the duration of the test. This stub is dynamically injected into PATH in test mode. So, for the purpose of the test it should not matter whether brew exists on the user's system or not. Our brew stub should take precedence.

So if this test fails on systems without brew for anyone, we shouldn't aim to fix a single test scenario and its output, but to fix the stub_repeated brew system that's used in tests. Thoughts?

This patch was originally created for the Debian package for ruby-build:

When was this patch created? Did you author it? Have you experienced the test failure in question?

@piperswe
Copy link
Contributor Author

Turns out I had forgotten the context - the patch is a few years old at this point. It's not actually anything to do with Homebrew, and I can remove that part. The Debian build environment doesn't have the .gitdirectory. I'll update this PR to only check for the existence of .git - I think it's reasonable to want this test to pass when you download the tarball from GitHub instead of cloning.

@piperswe piperswe force-pushed the fix-nonexistent-version branch from ae2c92b to 5e21326 Compare August 27, 2024 18:54
@mislav mislav force-pushed the fix-nonexistent-version branch from 5e21326 to ba5e5a6 Compare January 21, 2025 20:46
Currently, this test fails if .git isn't present as it expects a
Git-specific output. This change temporarily creates a ".git"
directory during the test run to simulate the existence of a git
repository, thus keeping `rbenv install` output stable.

Co-authored-by: Mislav Marohnić <[email protected]>
@mislav mislav force-pushed the fix-nonexistent-version branch 2 times, most recently from a14fc11 to 434ce5f Compare January 21, 2025 21:04
@mislav
Copy link
Member

mislav commented Jan 21, 2025

Thanks for this and sorry for the late reaction. I've amended the approach in a way that I find easier to read/maintain: temporarily create the .git directory if missing.

@mislav mislav merged commit 3643c8e into rbenv:master Jan 21, 2025
4 checks passed
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