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

Make tests pass for people who aren't Colin #39

Merged
merged 7 commits into from
Nov 10, 2022
Merged

Conversation

smcv
Copy link
Collaborator

@smcv smcv commented May 5, 2021

  • tests: Don't rely on git's default branch name

    The default branch name is configurable via the init.defaultBranch
    configuration option, so we can't count on it being master.
    Explicitly use a non-default branch so that it doesn't matter what
    the default is.

  • tests: Don't make assumptions about what will be pushed by default

    What to push by default is configurable via the push.default option,
    so we can't count on it pushing the current branch to the origin
    remote. Explicitly push the branch we're using.

  • tests: Configure git to allow file:/// submodules

    Git forbids file:/// for submodules by default, to avoid untrusted
    file:/// repositories being able to break a security boundary
    (CVE-2022-39253). However, this breaks git-evtag's test suite.

    In this test suite, all the repositories are under our control and
    we trust them, so bypass that restriction.

    Bug-Debian: https://bugs.debian.org/1023803

  • git-evtag-compute-py: Port to Python 3

    Python 2 reached EOL in January 2020.

  • tests: Configure git author/committer name

    Otherwise, recent versions of git will refuse to commit anything on an
    unconfigured system.

    Use Colin's name and email address here for now, rather than a dummy set
    of data. This is because the EVTag covers these, so if we don't use the
    same, we can't possibly get a match for the hard-coded expectation.

    The expectation for the new test added by git-evtag#36 needs to be
    adjusted for this, because it's comparing against a reference that
    was generated using Philip Withnall's name and email address. It seems
    simpler to use the same name and email address throughout the test suite.

  • tests: Compare C and Python implementations before making assertions

    It's helpful to be able to see the results of each implementation, and
    whether they agree, before comparing them with the reference.

  • tests: Run at build-time

    If we can catch bugs at build-time, then the package doesn't need to be
    included in a distribution so that its as-installed tests can run.

The default branch name is configurable via the init.defaultBranch
configuration option, so we can't count on it being `master`.
Explicitly use a non-default branch so that it doesn't matter what
the default is.

Signed-off-by: Simon McVittie <smcv@debian.org>
What to push by default is configurable via the push.default option,
so we can't count on it pushing the current branch to the origin
remote. Explicitly push the branch we're using.

Signed-off-by: Simon McVittie <smcv@debian.org>
Git forbids file:/// for submodules by default, to avoid untrusted
file:/// repositories being able to break a security boundary
(CVE-2022-39253). However, this breaks git-evtag's test suite.

In this test suite, all the repositories are under our control and
we trust them, so bypass that restriction.

Bug-Debian: https://bugs.debian.org/1023803
Signed-off-by: Simon McVittie <smcv@debian.org>
Python 2 reached EOL in January 2020.

Signed-off-by: Simon McVittie <smcv@debian.org>
Otherwise, recent versions of git will refuse to commit anything on an
unconfigured system.

Use Colin's name and email address here for now, rather than a dummy set
of data. This is because the EVTag covers these, so if we don't use the
same, we can't possibly get a match for the hard-coded expectation.

The expectation for the new test added by git-evtag#36 needs to be
adjusted for this, because it's comparing against a reference that
was generated using Philip Withnall's name and email address. It seems
simpler to use the same name and email address throughout the test suite.

Signed-off-by: Simon McVittie <smcv@debian.org>
It's helpful to be able to see the results of each implementation, and
whether they agree, before comparing them with the reference.

Signed-off-by: Simon McVittie <smcv@debian.org>
If we can catch bugs at build-time, then the package doesn't need to be
included in a distribution so that its as-installed tests can run.

Signed-off-by: Simon McVittie <smcv@debian.org>
@smcv
Copy link
Collaborator Author

smcv commented Nov 10, 2022

@cgwalters, please could you take a look at this? It's been a while without comment, and I'd really like to be able to run the tests for use in the Debian packaging. I've just updated the PR so tests can still pass with the latest git versions.

I'm not sure how @pwithnall was able to run the test suite for #36, since the first 6 tests expect to be run by Colin (and checks the tags against the expected sha512 for Colin's name and email address), whereas the new test for #36 expects to be run by Philip... but perhaps there's a trick?

@smcv
Copy link
Collaborator Author

smcv commented Nov 10, 2022

If it would be helpful, I can look at adding Github Workflows CI or a less Autotools'y build system as a follow-up to this.

Copy link
Owner

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much for doing this!

@cgwalters cgwalters merged commit 032175d into cgwalters:master Nov 10, 2022
@cgwalters
Copy link
Owner

If it would be helpful, I can look at adding Github Workflows CI or a less Autotools'y build system as a follow-up to this.

That'd be great!

@pwithnall
Copy link
Contributor

This is a victory for people everywhere not called Colin!

@smcv
Copy link
Collaborator Author

smcv commented Nov 11, 2022

If it would be helpful, I can look at adding Github Workflows CI or a less Autotools'y build system as a follow-up to this.

I've done the CI part; please take a look at my other PRs in this repository.

I haven't done a Meson build system yet, but I'll try to come back to that soon. It'll look something like the one in bubblewrap.

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.

3 participants