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

exec, tests: consolidate git procs #380

Merged
merged 13 commits into from
Jul 1, 2021

Conversation

ee7
Copy link
Member

@ee7 ee7 commented Jun 29, 2021

Summary:

  • Add a src/exec.nim file, containing helpers for startProcess.
  • Move the cloneExercismRepo proc there.
  • Add a setupExercismRepo proc for combined clone + checkout.
  • Start using these procs in the tests.

This PR is a follow-up of #379, where I wrote:

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.

See the individual commits.

The hard question: can we think of a better name than myExecCmdEx? I might use just exec instead - it's hard for a reader to confuse with nimscript.exec because that proc doesn't return anything.

A later PR will make src/probspecs.nim also use this shared code. And other things in tests/test_binary.nim too.

@ee7 ee7 requested a review from ErikSchierboom as a code owner June 29, 2021 11:29
src/exec.nim Outdated Show resolved Hide resolved
src/exec.nim Outdated Show resolved Hide resolved
ee7 added 5 commits June 29, 2021 14:04
I adapted this from my own code, where the `command` param is instead
`path` - the location of the executable - and `poUsePath` is not a
default option.
There is also proc named `exec` in the `nimscript` module, but that one
doesn't return a value.
It's more like "perform a shallow clone", not "is a shallow clone".
@ee7 ee7 merged commit 31ed8d3 into exercism:main Jul 1, 2021
@ee7 ee7 deleted the tests-refactor-exec-and-git branch July 1, 2021 06:58
@ee7 ee7 changed the title tests: refactor to share code for exec and git exec, tests: consolidate git procs Jul 1, 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