Skip to content

Tests: Ignore .github directories in package test #5098

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

JKLeckr
Copy link
Contributor

@JKLeckr JKLeckr commented Jun 10, 2025

What is this fixing or adding?

What is this?

The package test now ignores .github directories (can be changed to exclude more or less directories).

Why is it needed?

There are some APWorlds that use python scripts for GitHub Actions (contained in .github), but then doing so would fail the unit tests.
There is always adding an __init__.py in there, but then python would treat the .github directory as a module, which isn't always the desired behavior.

How was this tested?

Ran the Unit Tests with and without python scripts in the .github directories, and tested to see if it still caught .py scripts without __init__.py in other directories.

@github-actions github-actions bot added the waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. label Jun 10, 2025
@JKLeckr JKLeckr force-pushed the dev branch 2 times, most recently from 1117721 to 66ca041 Compare June 10, 2025 18:10
@JKLeckr JKLeckr changed the title Tests: Ignore .github dir in package test Tests: Ignore .github directories in package test Jun 10, 2025
Copy link
Contributor

@silasary silasary left a comment

Choose a reason for hiding this comment

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

Seems reasonable

@Exempt-Medic Exempt-Medic added is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. affects: core Issues/PRs that touch core and may need additional validation. labels Jun 11, 2025
@JKLeckr JKLeckr force-pushed the dev branch 2 times, most recently from b34d012 to f638053 Compare June 20, 2025 14:11
worlds_path = Utils.local_path("worlds")
for dirpath, dirnames, filenames in os.walk(worlds_path):
dirnames[:] = [d for d in dirnames if d not in ignore_dirs]
Copy link
Collaborator

Choose a reason for hiding this comment

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

this could probably use a comment as it's not obvious that it's modifying the list used in the walk generator

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Y'know. Ironically, I was taking a lot of time contemplating whether the comments were necessary or not since I did not see much existing comments in this code.
I added comments.

Added comments for ignore code
Copy link
Collaborator

@qwint qwint left a comment

Choose a reason for hiding this comment

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

code looks fine, lightly tested with different folder structures + invalid tests to see errors when it was included and it filtered as expected

@qwint qwint added waiting-on: core-review Issue/PR has been peer-reviewed and is ready to be merged or needs input from a core maintainer. and removed waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. labels Jun 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects: core Issues/PRs that touch core and may need additional validation. is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. waiting-on: core-review Issue/PR has been peer-reviewed and is ready to be merged or needs input from a core maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants