Skip to content

Port tests#336

Merged
technoweenie merged 14 commits intomasterfrom
port-tests
May 27, 2015
Merged

Port tests#336
technoweenie merged 14 commits intomasterfrom
port-tests

Conversation

@technoweenie
Copy link
Contributor

This moves all the tests from the commands package to the new shell tests.

  • clean
  • env
  • init - I removed these tests because they test that the global git config is updated. I don't want tests to modify existing global git configs.
  • ls-files
  • pointer
  • pre-push
  • push
  • smudge
  • status
  • update
  • version - also removed, since every test run calls this

@michael-k
Copy link
Contributor

I'm not happy with this. Shell scripts are horrible in many ways. Some arguments, why we shouldn't rely on shell scripts:

  • It should be fun to write tests, with shell scripts (for integration tests), it's not.
  • There is no way to see any coverage. (Yeah, there's rarely coverage for integration tests. This means, that we still need *_test.go files to run internal tests on the files in commands/.)
  • Learning shell scripting might be to big an obstacle for some developers.

If there's no way to make the *_test.go files work, I'd vote in favor of separate go files (e.g. one per integration test), executed with go run filename.go. We could still use exec where necessary and would be close to the functionality of shell scripts, but the code would be in go.

@technoweenie
Copy link
Contributor Author

Honestly, I'm not a huge fan of the shell tests either. I'd be down for support both sh and go tests so we can experiment with some single file go tests like you suggested. If we find a pattern that works, we can then move tests back to go one by one. But there's nothing about the current Go test stuff that I want to keep.

@technoweenie
Copy link
Contributor Author

FWIW here's another attempt at Go tests containing simpler declarative statements like the shell tests.

https://github.com/github/git-lfs/pull/291/files#diff-85b5fd21f7c36d3ba95b3a6fc0225941

I'm merging this so we can move forward on some other PRs. Still open to the idea of exploring single file go tests, though.

technoweenie added a commit that referenced this pull request May 27, 2015
@technoweenie technoweenie merged commit 7f11548 into master May 27, 2015
@technoweenie technoweenie deleted the port-tests branch May 27, 2015 19:57
@technoweenie technoweenie mentioned this pull request May 27, 2015
3 tasks
@michael-k
Copy link
Contributor

👍

chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Feb 20, 2024
The "git lfs ls-files" command was added in PR git-lfs#122 (technically,
the precursor "git media ls-files" command) and its test suite
converted to shell tests in PR git-lfs#336; however, the basic functionality
of the command has never had tests which confirm it handles files
in subdirectories appropriately.  (Some tests added in later PRs which
do check files in subdirectories under specific conditions, such as
with the --exclude option or with non-ASCII characters in subdirectory
names.)

As we expect to expand this command's test suite in subsequent commits
in this PR, we first add a new test which simply confirms that the
normal output of the command, and the output with the --debug option,
perform as expected when files have been both added and removed within
a subdirectory.

We also add a test which confirms the same behaviour when the --json
option is used.  The use of a separate test for this option was
preferred in PR git-lfs#5007 when the --json option was first introduced (rather
than overloading the existing tests, as was done for the --debug option
when it was added in PR git-lfs#2540), so we follow that model here as well.
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