Skip to content
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

test: Fix sitemap test base directory. #37

Merged
merged 2 commits into from
Nov 4, 2024

Conversation

projectgus
Copy link
Contributor

When run with "pdm run invoke test", the CWD is the root of the sitemap plugin (at least on Linux). However, the test case expects to find content in the same directory as the test source file.

Closes #36.

@projectgus
Copy link
Contributor Author

It looks like when the "CONTENT" directory didn't exist (it was /path/to/sitemap/test_data not /path/to/sitemap/pelican/plugins/sitemap/test_data), Pelican loaded content from its own tests/ directory which I think is why the errors relate to first-article.html generated from here - although I'm not sure why it loaded this file at all, or why it seems to have loaded it twice?

So I don't exactly understand why that behaviour appears, it might be a secondary bug that masked this one?

projectgus and others added 2 commits November 4, 2024 17:02
When run with "pdm run invoke test", the CWD is the root of the sitemap
plugin. However, the test case expects to find content in the same
directory as the test source file.
Copy link
Contributor

@justinmayer justinmayer left a comment

Choose a reason for hiding this comment

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

Many thanks, Angus! Interesting… I'm not sure about the questions you raised, but it seems that you have found a solution that will suit our needs for the moment.

I took the liberty of replacing the __file__ reference with the importlib.resources.files Python 3.9+ API.

Thanks again for your help. Much appreciated! 🏅

@justinmayer justinmayer merged commit 89f32c2 into pelican-plugins:main Nov 4, 2024
7 checks passed
@projectgus
Copy link
Contributor Author

I took the liberty of replacing the __file__ reference with the importlib.resources.files Python 3.9+ API.

Oh nice, I did catch myself wondering if there was a more modern way to get this information - and now I know what it is!

@projectgus projectgus deleted the bugfix/sitemap_tests branch November 4, 2024 22:32
@@ -24,6 +25,7 @@ def tearDown(self):
def _run_pelican(self, sitemap_format):
settings = read_settings(
override={
"PATH": BASE_DIR,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh snap, I figured the root cause!

Settings key CONTENT isn't used at all I don't think (maybe it was in an old version?), the key is PATH here and the default is ".".

So if you run the tests from the top-level sitemap directory, and you have a .venv in the top-level directory (i.e. the case for CI and when I tested), then Pelican recursively reads all the content installed in the virtualenv which includes all the pelican package's test files!

And because the lib64 directory is symlinked to lib on Linux, you get two copies of each file which is how this error cropped up when it tried to write the same output file twice

❯ find .venv -name first-article.rst
.venv/lib/python3.12/site-packages/pelican/tests/cyclic_intersite_links/first-article.rst

❯ find -L .venv -name first-article.rst
.venv/lib/python3.12/site-packages/pelican/tests/cyclic_intersite_links/first-article.rst
.venv/lib64/python3.12/site-packages/pelican/tests/cyclic_intersite_links/first-article.rst

I see the default for IGNORED_FILES in Pelican is .#* but maybe it should be .* to avoid all "hidden" dirs?

projectgus added a commit to projectgus/sitemap that referenced this pull request Nov 10, 2024
Follow-up to 3186431, as per comment here:
pelican-plugins#37 (comment)

- CONTENT key is unused
- PATH key determines the content path
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.

Sitemap plugin test errors in CI
2 participants