Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Port all tests to clar #615

Merged
merged 19 commits into from

2 participants

@ben

Having two test suites is a drag. I see in #406 that there was some effort towards moving the test suite over to Clar (then Clay), but it looks like it was never finished.

I've tried to be idiomatic to Clar in how the tests have been converted, with __initialize and __cleanup calls, and using static strings rather than #defines wherever possible. I've also split them up from their original suites; take a look at the individual commits and their comments if you'd like to see which Clar tests came from which old suites.

I'm sure I did something wrong along the way, though, and I'd be happy to fix it.

@vmg
Owner

Wow Ben, amazing job. Thank you so much!

Here's a couple things:

  1. The most idiomatic thing you missed is that in Clar, we never return error codes from auxiliary functions in the test suite. Instead, we just assert the values inside of the method, which makes the test suite much easier to read. I'll comment on the diff with examples.

  2. Could you remove all the tests that have been successfully ported from the tests folder? That way we can keep track of what's missing.

tests-clar/refs/normalize.c
@@ -0,0 +1,197 @@
+#include "clar_libgit2.h"
+
+#include "repository.h"
+#include "git2/reflog.h"
+#include "reflog.h"
+
+
+// Helpers
+static int ensure_refname_normalized(int is_oid_ref, const char *input_refname, const char *expected_refname)
@vmg Owner
vmg added a note

This is a very obvious example. Here's how I'd write it:

static void ensure_refname_normalized(int is_oid_ref, const char *input_refname, const char *expected_refname)
{
    char buffer_out[GIT_REFNAME_MAX];

    if (is_oid_ref)
        cl_git_pass(git_reference__normalize_name_oid(buffer_out, sizeof(buffer_out), input_refname));
    else
        cl_git_pass(git_reference__normalize_name(buffer_out, sizeof(buffer_out), input_refname));

    cl_assert(strcmp(buffer_out, expected_refname) == 0);
}

static void ensure_refname_invalid(int is_oid_ref, const char *input_refname)
{
    char buffer_out[GIT_REFNAME_MAX];

    if (is_oid_ref)
        cl_git_fail(git_reference__normalize_name_oid(buffer_out, sizeof(buffer_out), input_refname));
    else
        cl_git_fail(git_reference__normalize_name(buffer_out, sizeof(buffer_out), input_refname));
}

This way we can skip the cl_git_fail and cl_git_pass wrappers when calling the functions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
tests-clar/refs/reflog.c
((1 lines not shown))
+#include "clar_libgit2.h"
+
+#include "repository.h"
+#include "git2/reflog.h"
+#include "reflog.h"
+
+
+static const char *new_ref = "refs/heads/test-reflog";
+static const char *current_master_tip = "a65fedf39aefe402d3bb6e24df4d4f5fe4547750";
+static const char *commit_msg = "commit: bla bla";
+
+static git_repository *g_repo;
+
+
+// helpers
+static int assert_signature(git_signature *expected, git_signature *actual)
@vmg Owner
vmg added a note

Likewise.

static void assert_signature(git_signature *expected, git_signature *actual)
{
    cl_assert(actual);
    cl_assert(strcmp(expected->name, actual->name) == 0));
    cl_assert(strcmp(expected->email, actual->email) == 0));
    cl_assert(expected->when.offset == actual->when.offset);
    cl_assert(expected->when.time == actual->when.time);
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
ben added some commits
@ben ben Ref normalization test helpers now doing internal asserts. 8e82600
@ben ben Removing test suites that have been ported to Clar:
* t00 / 6e86fb3 (mentioned in #406)
* t01 / fc60c4a (mentioned in #406)
* t03 / bcbabe6
* t04 / PR #606
* t05 / d96f2c3
* t06 / 6c106ee
* t07 / 2ef582b
* t08 / b482c42
* t09 / 9a39a36
* t10 / 00a4893
* t12 / 7c3a4a7
* t13 / 1cb9b31
* t17 / cdaa6ff (mentioned in #406)
* t18 / efabc08
9297b6e
@ben

Sort of like this? I mean, I just copied-and-pasted what you did. :) I'll scrub the rest in a bit.

I tried to document the commit where each test suite was ported. This leaves the tests directory rather empty, and it doesn't actually build, so it's probably time to move resources into tests-clar and delete the old directory entirely.

@vmg
Owner

Looking good. ^^

Keep an eye out: there's still other auxiliary functions that can be stopped from having return codes. Also, if there are no tests left in the test, do kill it.

@ben

Move is done, and I've fixed all of the test helpers I could find with ack "static [^v]". There are probably some that will slip through this filter; I'll take a gander after the kids go to bed.

@ben
ben commented

I scrubbed all the clar test suites looking for helpers that were needlessly non-void. It looks pretty clean, so... what did I miss?

@vmg
Owner
vmg commented

:metal:

@vmg vmg merged commit 4cba39a into libgit2:development
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 30, 2012
  1. @ben

    t03_objwrite.c ported.

    ben authored
  2. @ben

    t06_index.c ported.

    ben authored
  3. @ben

    t07_hashtable.c ported.

    ben authored
  4. @ben

    t08_tag.c ported.

    ben authored
    Also cleaned up some names for things that used to be macros.
  5. @ben
  6. @ben
  7. @ben
  8. @ben

    t09-tree.c ported.

    ben authored
  9. @ben

    Moved tag tests to object suite.

    ben authored
  10. @ben

    Fixed linux build/test issues.

    ben authored
  11. @ben

    t10-refs.c ported.

    ben authored
  12. @ben

    t12-repo.c ported, although kind of messy.

    ben authored
    It'd be nice to be rid of all the #define's, but converting to const
    char*'s would be even messier, and less declarative.
  13. @ben

    t13-threads.c ported.

    ben authored
Commits on Mar 31, 2012
  1. @ben
  2. @ben

    Removing test suites that have been ported to Clar:

    ben authored
    * t00 / 6e86fb3 (mentioned in #406)
    * t01 / fc60c4a (mentioned in #406)
    * t03 / bcbabe6
    * t04 / PR #606
    * t05 / d96f2c3
    * t06 / 6c106ee
    * t07 / 2ef582b
    * t08 / b482c42
    * t09 / 9a39a36
    * t10 / 00a4893
    * t12 / 7c3a4a7
    * t13 / 1cb9b31
    * t17 / cdaa6ff (mentioned in #406)
    * t18 / efabc08
  3. @ben
  4. @ben

    Moved testing resources to clar, and removed old tests directory.

    ben authored
    Removed the BUILD_CLAR CMake flag, and updated the readme.
Commits on Apr 1, 2012
  1. @ben
  2. @ben

    Simple readability fixes.

    ben authored
Something went wrong with that request. Please try again.