Exclude the appmap test files#271
Conversation
| exclude = ['_appmap/wrapt'] | ||
| exclude = [ | ||
| '_appmap/wrapt', | ||
| '_appmap/test/data/*/appmap.yml' |
There was a problem hiding this comment.
Maybe all of _appmap/test would be fine?
|
It's important to ensure that all the files necessary to run the tests are included in the source distribution (sdist) that gets created by poetry (and uploaded to the PyPI). If they are, then this change is fine, and in fact can be improved by Kevin's suggestion of excluding all of If they're not included, though, you'll need to dig into the poetry doc and see if there's a different approach. |
|
Also, the Travis build is red. Looks like you may need to pin pytest-django to an older version. |
So there are different types of distributions? The package distribution and the source distribution? |
|
Thanks @apotterri i'll check out some docs to see what some ideas could be to solve this. I suppose if there isn't an easy way to exclude those files then the alternative solution would be for me to just modify the tests to use an appmap.yml with a different name and just change those filenames accordingly? |
@petecheslock what I think he is saying is that there is more than one kind of Python distribution. For example "package" distribution, and "source" distribution, which are similar, but have different purposes. @apotterri please clarify Renaming the appmap.yml files is a last resort if this can't be solved via packaging. |
|
Yea i think that is correct. I found this in the poetry project which gives an example of how i might exclude those files from the wheel distribution while also keeping them included in the source. python-poetry/poetry#3380 (comment) going to give this a try and update this PR when i have it working. |
|
Ok - i think i have this working. With the recent change i do the following
From there i get this: if i unzip the source tar.gz the When i unzip the wheel - the test directory does NOT exist. Then in my django oscar project i install the wheel from the local built version Then after install i checked the log and only one indexer is running (which is what is expected). |
Yup, this is the desired behavior. 👍 |
|
Thanks @apotterri - i updated the files (i thiink correctly) to use a working version on pytest-django and the tests now pass. Can you let me know if that is correct and then this is ready for review. |
apotterri
left a comment
There was a problem hiding this comment.
LGTM. Thanks for doing this.
* Exclude the appmap test files * Update glob to exclude more files * Include the necessary files only for the sdist exclude them otherwise * Use a lower working version of pytest-django * Update the tox config correctly
* Exclude the appmap test files * Update glob to exclude more files * Include the necessary files only for the sdist exclude them otherwise * Use a lower working version of pytest-django * Update the tox config correctly
## [1.18.2](v1.18.1...v1.18.2) (2024-01-22) ### Bug Fixes * Exclude the appmap test files ([#271](#271)) ([9ff91b8](9ff91b8))
Taking a first pass of this - see the issue here: getappmap/appmap-js#1555. AppMap spawns separate index files for every appmap.yaml - so excluding the test data from the package.
Fixes #272