Bump coverage.py to v7.3.0 in ansible-test

[webknjaz]

This patch upgrades the coverage.py version used under Python 3.8 through 3.12.
It also upgrades the path patterns in the generated configs according to the changes
in v7.0.0b1 of said library as it replaced the matching method from fnmatch to glob[1][2].

Resolves #80427

SUMMARY

^ $sbj ^

ISSUE TYPE

• Maintenance Pull Request
• Test Pull Request

COMPONENT NAME

ansible-test

ADDITIONAL INFORMATION

N/A

[mattclay]

@webknjaz The changes looked correct, but I decided to test the glob matching to be sure. At first I tried reverting the glob portion of your changes in this PR -- yet code coverage was still reported. Then I tried using the API directly and I'm seeing the old behavior:

from coverage.files import GlobMatcher

print(GlobMatcher('/hello/*').match('/hello/too/deep/to/match.py'))

I'd expect that not to match, since I didn't use **, but it does. Can you confirm that glob matching is (or is not) working as documented?

[mattclay]

Also, once we've merged this PR with whatever coverage version we pick, we'll want to update the spare-tire repo:

https://github.com/ansible/spare-tire/blob/b23b16c077ce0e8ada65c12b79a608025d4fb1fd/wheel_matrix.yml#L94

We should be able to just replace 6.5.0 with the new pinned version. The purpose of that is to ensure that if we add a new platform/version later, that it will get a wheel build for the pinned version (not just latest). We shouldn't need to keep the old entry around though, since we won't be adding any new platform/version combinations using the old version.

[webknjaz]

from coverage.files import GlobMatcher

print(GlobMatcher('/hello/*').match('/hello/too/deep/to/match.py'))

I'd expect that not to match, since I didn't use **, but it does. Can you confirm that glob matching is (or is not) working as documented?

@mattclay I think you're initializing it wrong. Look at the initializer signature:

class GlobMatcher:
    """A matcher for files by file name pattern."""
    def __init__(self, pats: Iterable[str], name: str = "unknown") -> None:
        self.pats = list(pats)
        self.re = globs_to_regex(self.pats, case_insensitive=env.WINDOWS)
        self.name = name

It expects an iterable of strings and applies list() to it. So you're actually supplying a list of one-char globs rather than one:

In [18]: list('/hello/*')
Out[18]: ['/', 'h', 'e', 'l', 'l', 'o', '/', '*']

Unsurprisingly, it doesn't do what you meant. Wrapping it into a non-string iterable should fix that, but it still matches:

In [19]: GlobMatcher('/hello/*').match('/hello/too/deep/to/match.py')
Out[19]: True

In [20]: GlobMatcher(('/hello/*', )).match('/hello/too/deep/to/match.py')
Out[20]: True

These are the internal regexps it creates out of the args:

In [27]: GlobMatcher('/hello/*').re
Out[27]: 
re.compile(r'(?:(?:[/\\])|(?:(.*[/\\])?h)|(?:(.*[/\\])?e)|(?:(.*[/\\])?l)|(?:(.*[/\\])?l)|(?:(.*[/\\])?o)|(?:[/\\])|(?:(.*[/\\])?[^/\\]*))\Z',
           re.UNICODE)

In [28]: GlobMatcher(('/hello/*', )).re
Out[28]: re.compile(r'(?:[/\\]hello[/\\].*)\Z', re.UNICODE)

Need to look deeper...

[webknjaz]

It looks like /* at the very end of a pattern is a corner case, specifically defined here: nedbat/coveragepy@ec6205a#diff-fd153710f7f1f017fbf6798fb808fc2852d59ccab90f6393e010d740d30ef804R292.
In this regard, it does not behave like https://docs.python.org/3/library/glob.html.

In [31]: GlobMatcher(('/hello/*/thing/', )).re
Out[31]: re.compile(r'(?:[/\\]hello[/\\][^/\\]*[/\\]thing[/\\])\Z', re.UNICODE)

In [32]: GlobMatcher(('/hello/*/', )).re
Out[32]: re.compile(r'(?:[/\\]hello[/\\][^/\\]*[/\\])\Z', re.UNICODE)

In [33]: GlobMatcher(('/hello/*', )).re
Out[33]: re.compile(r'(?:[/\\]hello[/\\].*)\Z', re.UNICODE)

In [34]: GlobMatcher(('/hello/*/thing/', )).match('/hello/too/deep/to/match.py')
Out[34]: False

In [35]: GlobMatcher(('/hello/*/too/', )).match('/hello/too/deep/to/match.py')
Out[35]: False

In [36]: GlobMatcher(('/hello/*/deep/', )).match('/hello/too/deep/to/match.py')
Out[36]: False

In [37]: GlobMatcher(('/hello/*/deep/*', )).match('/hello/too/deep/to/match.py')
Out[37]: True

In [38]: GlobMatcher(('/hello/*/', )).match('/hello/too/deep/to/match.py')
Out[38]: False

In [39]: GlobMatcher(('/hello/*', )).match('/hello/too/deep/to/match.py')
Out[39]: True

In [40]: GlobMatcher(('/hello/*', )).match('/hello/too/')
Out[40]: True

In [41]: GlobMatcher(('/hello/*/', )).match('/hello/too/')
Out[41]: True

In [42]: GlobMatcher(('/hello/*/', )).match('/hello/too/deep')
Out[42]: False

In [43]: GlobMatcher(('/hello/*/', )).match('/hello/too/deep/')
Out[43]: False

It is unclear why this specific corner case was added, as it is not documented explicitly, nor does the commit add any tests for this case. It might be intended or a bug, there's no way to deduce it w/o asking..

[webknjaz]

@mattclay I submitted nedbat/coveragepy#1650 — let's see if that gets us the desired behavior in the end.

[mattclay]

Since there hasn't been any movement on nedbat/coveragepy#1650, I think we should move on without waiting for that. That appears to be safe to do, since the only issue seems to be that /* counts as /**, correct?

[webknjaz]

That appears to be safe to do, since the only issue seems to be that /* counts as /**, correct?

Yes, and I think that sticking ** everywhere like I've done in this patch is the way to go and doesn't need any special-casing, right?

[mattclay]

That appears to be safe to do, since the only issue seems to be that /* counts as /**, correct?

Yes, and I think that sticking ** everywhere like I've done in this patch is the way to go and doesn't need any special-casing, right?

Yes, it does appear to work fine with the older versions, and even though it isn't needed in all cases for the current versions, it does match the documented behavior. So we should be OK regardless of whether or not the issue we discovered is resolved.

Addressed.

[webknjaz]

just needs a changelog fragment.

Interestingly, I haven't found any changelog fragments coupled with the previous version bumps — I was hoping for an example of what could be expected.

[webknjaz]

@mattclay should we revert these after all?

[mattclay]

Just to confirm, the new ** behavior is only for matching in the middle of a path? The */ and /* matches at the start and end of a path continue to work without **?

If that's the case, then yes, we should revert the path changes, since it seems the coverage docs will be updated, rather than the implementation.

[webknjaz]

Yes, that's correct. I've been thinking of documenting this upstream, since it's unlikely that Ned will accept the original change I offered.

[mattclay]

just needs a changelog fragment.

Interestingly, I haven't found any changelog fragments coupled with the previous version bumps — I was hoping for an example of what could be expected.

The versions supported haven't changed since 2.14, when support for multiple coverage versions was first added in #77578. The version currently in use was set before the 2.14.0 release in #78945, so it was essentially already covered by the existing changelog entry.

[webknjaz]

FTR the docs are being improved here: nedbat/coveragepy#1675. In case, you have an opinion.

[webknjaz]

@mattclay this is ready!

[webknjaz]

/azp run

[azure-pipelines]

Pipelines were unable to run due to time out waiting for the pull request to finish merging.