Skip to content

Commit

Permalink
Baby steps towards making CmdUpdate unit testable
Browse files Browse the repository at this point in the history
As part of the work on #120, an error when using the update command,
I wanted to add tests to demonstrate the bug and then show that the
bug was fixed by my changes.

This is problematic because there are no real unit tests for any
commands.

This commit is the first step in providing better unit testability
of various parts of corral in general and commands specifically.

The only test added by this commit is that when using a corral.json
with empty deps, that no VCS operations will be executed by _Updater.

There are a number of changes that are brought in my this initial refactor.
As we move forward with making corral more testable, some of these changes
will probably fall away. I did what I considered the minimal thing to get
a harness for testing _Updater working. This harness should be enough to
test using the various test-data bundles that we have as well as testing
issue #120.

The approach I have taken does leave a lot of things in an inconsistent state.
For example, this commit introduces a new way of organizing unit tests that keeps
said tests near the code they are testing and allows for testing private implementations
like the _Update that is part of the update cmd.

These inconsistencies are temporary and as we move forward with adding more tests, will
start to fall away.
  • Loading branch information
SeanTAllen committed May 25, 2020
1 parent 818cddb commit 7f00b2b
Show file tree
Hide file tree
Showing 17 changed files with 237 additions and 18 deletions.
11 changes: 11 additions & 0 deletions corral/cmd/_test.pony
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
use "ponytest"

actor Main is TestList
new create(env: Env) =>
PonyTest(env, this)

new make() =>
None

fun tag tests(test: PonyTest) =>
_TestCmdUpdate.make().tests(test)
146 changes: 146 additions & 0 deletions corral/cmd/_test_cmd_update.pony
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
use "files"
use "ponytest"
use "../bundle"
use "../util"
use "../vcs"

actor _TestCmdUpdate is TestList
new make() =>
None

fun tag tests(test: PonyTest) =>
test(_TestEmptyDeps)

class iso _TestEmptyDeps is UnitTest
fun name(): String =>
"cmd/update/" + __loc.type_name()

fun apply(h: TestHelper) ? =>
"""
Verify that when using an corral.json for with empty deps, that there
are never any sync, tag query, or checkout operations executed.
"""
let auth = h.env.root as AmbientAuth
let log = Log(LvlNone, h.env.err, SimpleLogFormatter)
let fp: FilePath = _TestData.file_path_from(h, "empty-deps")?
let repo_cache = _TestRepoCache(auth)?
let ctx = Context(h.env, log, log, false, repo_cache)
let project = Project(auth, log, fp)
let bundle = Bundle.load(fp, log)?
let recorder = _OpsRecorder(h, 0, 0, 0)
let vcs_builder: VCSBuilder = _TestCmdUpdateVCSBuilder(recorder)

let updater = _Updater(ctx, project, consume bundle, vcs_builder, recorder)

// when updater is finished, it will send a `cmd_completed` message to
// _OpsRecorder which will trigger pass/fail
h.long_test(2_000_000_000)


actor _OpsRecorder is CmdResultReceiver
let _h: TestHelper

let _expected_sync: U64
let _expected_tag_query: U64
let _expected_checkout: U64

var _sync: U64 = 0
var _tag_query: U64 = 0
var _checkout: U64 = 0

new create(h: TestHelper, s: U64, tq: U64, c: U64) =>
_h = h
_expected_sync = s
_expected_tag_query = tq
_expected_checkout = c

be sync() =>
_sync = _sync + 1

be tag_query() =>
_tag_query = _tag_query + 1

be checkout() =>
_checkout = _checkout + 1

be cmd_completed() =>
_h.assert_eq[U64](_expected_sync, _sync)
_h.assert_eq[U64](_expected_tag_query, _tag_query)
_h.assert_eq[U64](_expected_checkout, _checkout)

_h.complete(true)


class val _TestCmdUpdateVCSBuilder is VCSBuilder
let _recorder: _OpsRecorder

new val create(recorder: _OpsRecorder) =>
_recorder = recorder

fun val apply(kind: String): VCS =>
_RecordedVCS(_recorder)


class val _RecordedVCS is VCS
let _recorder: _OpsRecorder

new val create(recorder: _OpsRecorder) =>
_recorder = recorder

fun val sync_op(next: RepoOperation): RepoOperation =>
_RecordedSync(_recorder, next)

fun val tag_query_op(receiver: TagListReceiver): RepoOperation =>
_RecordedTagQuery(_recorder, receiver)

fun val checkout_op(rev: String, next: RepoOperation): RepoOperation =>
_RecordedCheckout(_recorder, next)


class val _RecordedSync is RepoOperation
let _recorder: _OpsRecorder
let _next: RepoOperation

new val create(recorder: _OpsRecorder, next: RepoOperation) =>
_recorder = recorder
_next = next

fun val apply(repo: Repo) =>
_recorder.sync()
_next(repo)


class val _RecordedTagQuery is RepoOperation
let _recorder: _OpsRecorder
let _next: TagListReceiver

new val create(recorder: _OpsRecorder, next: TagListReceiver) =>
_recorder = recorder
_next = next

fun val apply(repo: Repo) =>
let tags: Array[String] iso = recover Array[String] end
_recorder.tag_query()
_next(repo, consume tags)


class val _RecordedCheckout is RepoOperation
let _recorder: _OpsRecorder
let _next: RepoOperation

new val create(recorder: _OpsRecorder, next: RepoOperation) =>
_recorder = recorder
_next = next

fun val apply(repo: Repo) =>
_recorder.checkout()
_next(repo)

primitive _TestData
fun file_path_from(h: TestHelper, subdir: String = ""): FilePath ? =>
let auth = h.env.root as AmbientAuth
FilePath(auth, "corral/test/testdata")?.join(subdir)?

primitive _TestRepoCache
fun apply(auth: AmbientAuth): FilePath ? =>
FilePath(auth,"_test_cmd_update_repo_cache")?
6 changes: 5 additions & 1 deletion corral/cmd/cmd_add.pony
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@ class CmdAdd is CmdType
version = cmd.option("version").string()
revision = cmd.option("revision").string()

fun apply(ctx: Context, project: Project, vcs_builder: VCSBuilder) =>
fun apply(ctx: Context,
project: Project,
vcs_builder: VCSBuilder,
result_receiver: CmdResultReceiver)
=>
ctx.uout.info(
"add: adding: " + locator + " " + version + " " + revision)

Expand Down
6 changes: 5 additions & 1 deletion corral/cmd/cmd_clean.pony
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@ class CmdClean is CmdType
// clean_corral
true

fun apply(ctx: Context, project: Project, vcs_builder: VCSBuilder) =>
fun apply(ctx: Context,
project: Project,
vcs_builder: VCSBuilder,
result_receiver: CmdResultReceiver)
=>
ctx.uout.info(
"clean: corral:" + clean_corral.string() +
" repos:" + clean_repos.string())
Expand Down
6 changes: 5 additions & 1 deletion corral/cmd/cmd_fetch.pony
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@ class CmdFetch is CmdType

new create(cmd: Command) => None

fun apply(ctx: Context, project: Project, vcs_builder: VCSBuilder) =>
fun apply(ctx: Context,
project: Project,
vcs_builder: VCSBuilder,
result_receiver: CmdResultReceiver)
=>
ctx.uout.info("fetch: fetching from " + project.dir.path)

match project.load_bundle()
Expand Down
6 changes: 5 additions & 1 deletion corral/cmd/cmd_info.pony
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@ class CmdInfo is CmdType

new create(cmd: Command) => None

fun apply(ctx: Context, project: Project, vcs_builder: VCSBuilder) =>
fun apply(ctx: Context,
project: Project,
vcs_builder: VCSBuilder,
result_receiver: CmdResultReceiver)
=>
ctx.uout.info("info: from " + project.dir.path)

match project.load_bundle()
Expand Down
6 changes: 5 additions & 1 deletion corral/cmd/cmd_init.pony
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@ class CmdInit is CmdType
fun requires_bundle(): Bool => false
fun requires_no_bundle(): Bool => true

fun apply(ctx: Context, project: Project, vcs_builder: VCSBuilder) =>
fun apply(ctx: Context,
project: Project,
vcs_builder: VCSBuilder,
result_receiver: CmdResultReceiver)
=>
ctx.uout.info("init: in " + project.dir.path)

// TODO: try to read first to convert/update existing file(s)
Expand Down
6 changes: 5 additions & 1 deletion corral/cmd/cmd_list.pony
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@ class CmdList is CmdType

new create(cmd: Command) => None

fun apply(ctx: Context, project: Project, vcs_builder: VCSBuilder) =>
fun apply(ctx: Context,
project: Project,
vcs_builder: VCSBuilder,
result_receiver: CmdResultReceiver)
=>
ctx.uout.info("list: from " + project.dir.path)

match project.load_bundle()
Expand Down
6 changes: 5 additions & 1 deletion corral/cmd/cmd_remove.pony
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@ class CmdRemove is CmdType
new create(cmd: Command) =>
locator = cmd.arg("locator").string()

fun apply(ctx: Context, project: Project, vcs_builder: VCSBuilder) =>
fun apply(ctx: Context,
project: Project,
vcs_builder: VCSBuilder,
result_receiver: CmdResultReceiver)
=>
ctx.uout.info("remove: removing: " + locator)

match project.load_bundle()
Expand Down
6 changes: 5 additions & 1 deletion corral/cmd/cmd_run.pony
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@ class CmdRun is CmdType

fun requires_bundle(): Bool => false

fun apply(ctx: Context, project: Project, vcs_builder: VCSBuilder) =>
fun apply(ctx: Context,
project: Project,
vcs_builder: VCSBuilder,
result_receiver: CmdResultReceiver)
=>
ctx.uout.info("run: " + " ".join(args.values()))

// Build a : separated path from bundle roots.
Expand Down
5 changes: 4 additions & 1 deletion corral/cmd/cmd_type.pony
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,7 @@ trait CmdType
fun requires_bundle(): Bool => true
fun requires_no_bundle(): Bool => false

fun ref apply(ctx: Context, project: Project, vcs_builder: VCSBuilder)
fun ref apply(ctx: Context,
project: Project,
vcs_builder: VCSBuilder,
results_receiver: CmdResultReceiver)
21 changes: 16 additions & 5 deletions corral/cmd/cmd_update.pony
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,18 @@ use "../util"
use "../vcs"

class CmdUpdate is CmdType
new create(cmd: Command) =>
None

new create(cmd: Command) => None

fun apply(ctx: Context, project: Project, vcs_builder: VCSBuilder) =>
fun apply(ctx: Context,
project: Project,
vcs_builder: VCSBuilder,
results_receiver: CmdResultReceiver)
=>
ctx.uout.info("update: updating from " + project.dir.path)
match project.load_bundle()
| let bundle: Bundle iso =>
_Updater(ctx, project, consume bundle, vcs_builder)
_Updater(ctx, project, consume bundle, vcs_builder, results_receiver)
| let err: Error =>
ctx.uout.err(err.message)
ctx.env.exitcode(1)
Expand All @@ -29,16 +33,19 @@ actor _Updater
let dep_tags: Map[Locator, Array[String] val] ref = dep_tags.create()

let _vcs_builder: VCSBuilder
let _results_receiver: CmdResultReceiver

new create(ctx': Context,
project': Project,
base_bundle': Bundle iso,
vcs_builder: VCSBuilder)
vcs_builder: VCSBuilder,
results_receiver: CmdResultReceiver)
=>
ctx = ctx'
project = project'
base_bundle = consume base_bundle'
_vcs_builder = vcs_builder
_results_receiver = results_receiver
ctx.log.info("Updating direct deps of project bundle: " + base_bundle.name())
load_bundle_deps(base_bundle)

Expand Down Expand Up @@ -67,6 +74,9 @@ actor _Updater
// It won't get a lock. How should we handle/report the error?
end
end
if deps_to_load.size() == 0 then
_results_receiver.cmd_completed()
end

fun load_dep(dep: Dep) ? =>
let vcs = _vcs_builder(dep.vcs())?
Expand Down Expand Up @@ -140,6 +150,7 @@ actor _Updater
else
ctx.uout.err("Error saving project bundle")
end
_results_receiver.cmd_completed()
// All done, should quiesce now...
ctx.log.fine("try_complete all done.")
end
Expand Down
6 changes: 5 additions & 1 deletion corral/cmd/cmd_version.pony
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,9 @@ class CmdVersion is CmdType

fun requires_bundle(): Bool => false

fun apply(ctx: Context, project: Project, vcs_builder: VCSBuilder) =>
fun apply(ctx: Context,
project: Project,
vcs_builder: VCSBuilder,
result_receiver: CmdResultReceiver)
=>
ctx.uout.info("version: " + Version())
4 changes: 2 additions & 2 deletions corral/cmd/executor.pony
Original file line number Diff line number Diff line change
Expand Up @@ -103,5 +103,5 @@ primitive Executor

let context = Context(env, log, uout, nothing, repo_cache)
let project = Project(auth, log, bundle_dir)
let vcs_builder = VCSBuilder(env)
command(context, project, vcs_builder)
let vcs_builder = CorralVCSBuilder(env)
command(context, project, vcs_builder, NoOpResultReceiver)
6 changes: 6 additions & 0 deletions corral/cmd/result_receiver.pony
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
interface tag CmdResultReceiver
be cmd_completed()

primitive NoOpResultReceiver
fun tag cmd_completed() =>
None
3 changes: 3 additions & 0 deletions corral/test/_test.pony
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use "ponytest"
use "files"
use integration = "integration"
use cmd = "../cmd"

actor Main is TestList
new create(env: Env) =>
Expand All @@ -23,3 +24,5 @@ actor Main is TestList
test(integration.TestRun)
test(integration.TestRunWithoutBundle)
test(integration.TestClean)

cmd.Main.make().tests(test)
5 changes: 4 additions & 1 deletion corral/vcs/vcs_builder.pony
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
class val VCSBuilder
interface val VCSBuilder
fun val apply(kind: String): VCS ?

class val CorralVCSBuilder
let _env: Env

new val create(env: Env) =>
Expand Down

0 comments on commit 7f00b2b

Please sign in to comment.