Skip to content

Conversation

@cuu508
Copy link
Contributor

@cuu508 cuu508 commented Mar 10, 2025

What do these changes do?

This change hopefully fixes test
aiosmtpd.qa.test_0packaging.TestVersion.test_ge_master.

The test runs git show master:aiosmtpd/__init__.py command, and the command
fails if the master branch does not exist locally.

Are there changes in behavior for the user?

No, they just fixes CI to not skip a test.

Needed for aiosmtpd.qa.test_0packaging.TestVersion.test_ge_master,
it runs a "git show" command which will fail if the master branch
does not exist locally
@codecov
Copy link

codecov bot commented Mar 10, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.78%. Comparing base (f40ac96) to head (bb0b885).
⚠️ Report is 44 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #545      +/-   ##
==========================================
- Coverage   97.86%   97.78%   -0.08%     
==========================================
  Files          23       23              
  Lines        5707     5697      -10     
  Branches      764      764              
==========================================
- Hits         5585     5571      -14     
- Misses         76       80       +4     
  Partials       46       46              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cuu508
Copy link
Contributor Author

cuu508 commented Mar 11, 2025

A little more background on this. The test_ge_master test case looks like so:

    # noinspection PyUnboundLocalVariable
    def test_ge_master(
        self, aiosmtpd_version: version.Version, capsys: pytest.CaptureFixture
    ):
        """Ensure version is monotonically increasing"""
        reference = "master:aiosmtpd/__init__.py"
        cmd = f"git show {reference}".split()
        try:
            with capsys.disabled():
                master_smtp = subprocess.check_output(cmd).decode()  # nosec
        except subprocess.CalledProcessError:
            pytest.skip("Skipping due to git error")

        try:
            m = next(m for ln in master_smtp.splitlines() if (m := RE_DUNDERVER.match(ln)))
        except StopIteration:
            pytest.fail(f"Cannot find __version__ in {reference}!")
        master_ver = version.parse(m.group("ver"))
        assert aiosmtpd_version >= master_ver, "Version number cannot be < master's"

As I understand it, it grabs aiosmtpd/__init__.py from the master branch and makes sure the version number in the working copy is not lower than in the master.

When GHA runs tests on a pull request, only the pull request's branch is available. So when git show runs, it exits with exit code 128. The test code catches the exception and skips the test (Skipping due to git error).

I'm not sure if it makes sense to check if the version increases monotonically in PRs. If it does, this PR fixes the problem: it adds an extra command in ci-cd.yml to fetch the master branch. If it does not, then this PR can be discarded.

The reason I looked into this in the first place: I was looking into why test coverage was decreasing in a pull request I was working on. The coverage report showed regressions in a few places. One of them was this test. If the git show command fails then the test case bails out early, and the following lines are not covered:

        try:
            m = next(m for ln in master_smtp.splitlines() if (m := RE_DUNDERVER.match(ln)))
        except StopIteration:
            pytest.fail(f"Cannot find __version__ in {reference}!")
        master_ver = version.parse(m.group("ver"))
        assert aiosmtpd_version >= master_ver, "Version number cannot be < master's"

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.

1 participant