Verify package data in setup.py installs all files

[sivel]

SUMMARY

Verify package data in setup.py installs all files

We recently added a new data file and it was not matched by package_data and thus not installed: #59452

This test will help ensure new data files are properly installed, as we have missed several in the past.

ISSUE TYPE

• Feature Pull Request

COMPONENT NAME

test/sanity/code-smell/package-data.py

ADDITIONAL INFORMATION

before the change to setup.py:

ERROR: Found 1 package-data issue(s) which need to be resolved:
ERROR: lib/ansible/galaxy/data/default/role/tests/inventory:0:0: File not installed
ERROR: The 1 sanity test(s) listed below (out of 1) failed. See error output above for details.

[ansibot]

cc @mattclay
click here for bot help

[ansibot]

cc @mattclay
click here for bot help

[ansibot]

cc @acozine @gundalow
click here for bot help

[mattclay]

This test needs to be added to https://github.com/ansible/ansible/blob/devel/test/sanity/code-smell/ansible-only.txt since it should not be run on collections.

Also, I don't particularly like having an always running sanity test that runs for 10+ seconds, since it will more than double the time it takes to run sanity tests on a single file.

One option would be to parse package_data from setup.py and check that, rather than performing an actual install. It would be very fast, and nearly as good as performing an actual install. That may be good enough, unless you're aware of scenarios where that is likely to miss something.

[mattclay]

Suggested change

	if os.path.splitext(path)[-1] not in ('.py', '.pyc', '.pyo'):
	if os.path.splitext(path)[1] not in ('.py', '.pyc', '.pyo'):

[mattclay]

I'd prefer using os.path functions for path manipulation instead of string slicing.

[mattclay]

I'd prefer os.path.join here.

It's not a big issue though -- we certainly have plenty of other places where string concatenation is done.

[ansibot]

cc @acozine @gundalow
click here for bot help

[mattclay]

@sivel I discussed this with @nitzmahone and he pointed out that if entries are not correct in MANIFEST.in then we could miss things by looking only at package_data -- so it seems the only safe thing to do is run the install as you're already doing.

To avoid unnecessary overhead, lets treat this like we do the docs-build sanity test and make it disabled by default. It can be disabled like this:

ansible/test/sanity/code-smell/docs-build.json

Line 2 in 97edfcc

"disabled": true,

Then enabled in CI here:

ansible/test/utils/shippable/sanity.sh

Line 20 in 97edfcc

2) options=(--test ansible-doc --test docs-build) ;;