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

New testsuite based on Git's test-lib.sh #220

Draft
wants to merge 155 commits into
base: main
Choose a base branch
from

Conversation

djpohly
Copy link

@djpohly djpohly commented May 19, 2017

Closes #218

I've taken the tests that were written using Bats and ported them to Git's testing harness. Advantages:

  • Testing harness used for Git itself
  • Harness written entirely in shell (removes dependency on Bats or Perl)
  • Harness provides repository creation commands (removes need for vcsh_testrepo, which IIRC was the sticking point for merging back in February)
  • Runs faster (on my box at least)

I'll keep tinkering and expanding but wanted to let you know this was ready and available.

The Perl scripts were only running shell commands anyway; this seemed a
more natural fit.  Since bash does not capture trailing newlines, there
is a change to the "vcsh status" check from 300-add.t to ensure the
empty line is/isn't printed according to the terseness setting.
What do you know, Git does this for us.
Let us hope there are no spaces in the user's path, because this will
allows us to set VCSH="bash vcsh", VCSH="dash vcsh", etc.
This will simplify use with a harness like prove.
Still works with bats.
@djpohly
Copy link
Author

djpohly commented Dec 22, 2017

@RichiH Still have interest in this? The current 165 tests are mergeable/usable/passing as-is (with 17 marked as expect_failure for existing issues).

The state each test should return to is having the vcsh repos are in
sync with their upstreams.  With these as test_setup, it actually makes
things more different when skipping/selecting tests.
Rather than check to see if core.excludesfile and core.attributesfile
are set, check to see if the data written to the agreed-upon ignore or
attributes file actually takes effect.  (Less reliance on internals.)
Redirections with >&, piping with |&, and brace expansion weren't
handled by dash.  Replace with portable equivalents.
These check to make sure that values from the environment don't
interfere with internal variables used by vcsh.
No need for this with test-lib.sh
This suite can be fleshed out with some clarification on how exactly
config files are supposed to work/be used.
@alerque
Copy link
Collaborator

alerque commented Mar 29, 2021

I'm a bit late to the party here so excuse my confusion, but can I ask what the motivation for this is? I could see wishing to move away from Perl, but why would some mataure test framework like bats be a detriment as a developer/CI dependency? If anything I would have looked into migrating to bats not away from it.

@djpohly Are you still interested in this? I don't want work to go to waste but I also want something that's actually going to be a long term benefit and at first blush I'm unclear on what the gain is.

@alerque alerque marked this pull request as draft March 29, 2021 22:17
@djpohly
Copy link
Author

djpohly commented Mar 29, 2021

There's no migrating to or away from Bats here; either testsuite is better than what vcsh currently has, which is essentially none. You'll notice I wrote both - I was simply trying a couple of options for more thorough testing to see if one worked better than the other, and I happened to try Bats first.

In terms of long-term benefit, however, I found it much easier to write solid tests with Git's test-lib.sh harness. Bats is pretty limited in what it does, and test-lib.sh is more feature-rich and mature, having been actively developed since Git 1.0 (2005). Convenience functions for working with Git repositories, like test_create_repo and test_commit, were especially helpful when trying to write proper, self-contained tests rather than depending on an existing remote repository to which you only have read-only access. Features like test_expect_failure are also generally useful for good testing practice, so that you can write a test immediately without having to wait for the feature to be implemented or bug to be fixed.

I'm sure that some of the relevant conveniences could be copied from test-lib.sh to a Bats library if that's what the devs want. Or they can throw it all out. Personally, it doesn't matter to me - I wrote tests so that I could potentially start working on refactoring vcsh, but since nothing was merged I moved on to other things.

@alerque
Copy link
Collaborator

alerque commented Mar 30, 2021

Thanks @djpohly that's helpful background for reviewing this.

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.

Introduce comprehensive test suite
2 participants