Skip to content

Rewrite worktree tests #628

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

Merged
merged 1 commit into from
Feb 28, 2023
Merged

Rewrite worktree tests #628

merged 1 commit into from
Feb 28, 2023

Conversation

jcouball
Copy link
Member

Your checklist for this pull request

🚨Please review the guidelines for contributing to this repository.

  • Ensure all commits include DCO sign-off.
  • Ensure that your contributions pass unit testing.
  • Ensure that your contributions contain documentation if applicable.

Description

Under some situations which were not entirely clear to me, the test_worktree.rb tests occasionally fail.

It ends up that this is because test_thread.rb tests leak setting the global ENV['GIT_DIR'] value.

This PR makes the following changes:

  • test_worktree.rb tests were rewritten and do not use the test fixture. This makes the tests much easier to understand since you can see the setup for each test.
  • The tests/files/worktree test fixture files are not needed so those files (270+ files!) were deleted.
  • In test_thread_safety.rb, a teardown was added to reset the environment variables. This is a stop gap measure until Git::Lib#command can be rewritten to use a method that does not depend on the global ENV variable (currently it uses backticks to run git commands).
  • In test_helper.rb, the Test::Unit::TestCase#in_temp_dir was changed to call File#realpath on the temp directory that it returns to mimic what this gem does internally.

Signed-off-by: bully <jcouball@yahoo.com>
@jcouball jcouball merged commit 1ccdbe0 into master Feb 28, 2023
@jcouball jcouball mentioned this pull request Mar 1, 2023
@jcouball jcouball deleted the fix_worktree_tests branch March 10, 2023 00:26
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.

1 participant