On Sat, Nov 09, 2024 at 07:41:09PM +0000, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <stolee@xxxxxxxxx>
>
> Add some tests based on the current behavior, doing interesting checks
> for different sets of branches, ranges, and the --boundary option. This
> sets a baseline for the behavior and we can extend it as new options are
> introduced.
>
> Store and output a 'batch_nr' value so we can demonstrate that the paths are
> grouped together in a batch and not following some other ordering. This
> allows us to test the depth-first behavior of the path-walk API. However, we
> purposefully do not test the order of the objects in the batch, so the
> output is compared to the expected output through a sort.
>
> It is important to mention that the behavior of the API will change soon as
> we start to handle UNINTERESTING objects differently, but these tests will
> demonstrate the change in behavior.
>
> Signed-off-by: Derrick Stolee <stolee@xxxxxxxxx>
> ---
Nice. I like the approach of implementing the API in a single commit,
and then demonstrating a trivial "caller" by way of a custom test
helper. I think that the artifact of having a test helper here is useful
on its own, but it also serves as a good example of how to use the API,
and provides something to actually test the implementation with.
I'm going to steal this pattern the next time I need to work on
something that necessitates a complex new API ;-).
> +static int emit_block(const char *path, struct oid_array *oids,
> + enum object_type type, void *data)
> +{
> + struct path_walk_test_data *tdata = data;
> + const char *typestr;
> +
> + switch (type) {
> + case OBJ_TREE:
> + typestr = "TREE";
> + tdata->tree_nr += oids->nr;
> + break;
> +
> + case OBJ_BLOB:
> + typestr = "BLOB";
> + tdata->blob_nr += oids->nr;
> + break;
> + default:
> + BUG("we do not understand this type");
> + }
I think you could write this as:
if (type == OBJ_TREE)
tdata->tree_nr += oids->nr;
else if (type == OBJ_BLOB)
tdata->blob_nr += oids->nr;
else
BUG("we do not understand this type");
typestr = type_name(type);
Which DRYs things up a bit and uses the type_name() helper. That will
give you strings like "tree" and "blob" instead of "TREE" and "BLOB",
but I'm not sure if the casing is important.
Thanks,
Taylor
