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

tests(binary): improve repo cloning/checkout #379

Merged
merged 7 commits into from
Jun 29, 2021

Conversation

ee7
Copy link
Member

@ee7 ee7 commented Jun 28, 2021

This PR makes it faster to re-run the tests of the configlet
executable. These tests checkout a known state of some Exercism repos,
but previously always ran git clone - even if the repos were already
present locally.

With this PR, the tests only run git clone when necessary. This
makes it easier to add new tests - a configlet developer would otherwise
want to comment-out the git clone lines while doing so.

Other changes:

  • Simplify cloneExercismRepo (avoid our dirty template and block).
  • Print a message when cloning, so that it doesn't seem like a hang.
  • Make binaryPath const, to better signal to the reader that it is
    known at compile-time.
  • Download repos to the tests dir, rather than the repo root.
  • Add the elixir track clone path to gitignore - it was missing.

Future PRs will refactor this code further.


I was going to push these changes (and much more upcoming) to #366, but it makes that PR (and eventual squashed commit) too large - it'll already be pretty big. So I want to merge a few PRs first - this won't slow things down more. I consider adding decent tests one of the biggest parts of #366, so in this case I want to not hate the initial state of the new tests.

The next PR will improve the cloning process further, so we'll just have setupExercismRepo at the call site that does "clone if necessary, and checkout".

I think I've finally figured out an ergonomic design for some exec/git helper commands, so they'll soon infect the rest of our codebase too.

ee7 added 5 commits June 28, 2021 19:15
Avoid use of `block` and `dirty` template.
Otherwise it can appear like a hang.
This better signals to the reader that `binaryPath` is known at
compile-time.
@ee7 ee7 requested a review from ErikSchierboom as a code owner June 28, 2021 17:33
tests/test_binary.nim Show resolved Hide resolved
tests/test_binary.nim Show resolved Hide resolved
tests/test_binary.nim Outdated Show resolved Hide resolved
tests/test_binary.nim Outdated Show resolved Hide resolved
ee7 added 2 commits June 29, 2021 12:16
This isn't necessary, but it helps to remind the reader where they are.

(Also, order alphabetically).
@ee7 ee7 merged commit d005e29 into exercism:main Jun 29, 2021
@ee7 ee7 deleted the tests-binary-improve-cloning branch June 29, 2021 10:52
@ee7 ee7 changed the title tests(binary): improve cloning tests(binary): improve repo cloning/checkout Jun 29, 2021
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