[Inner loop] Require langdetect when running tests

[citelao]

Description

Fixes #5797.

Today, local tests (poe test) will fail to run if langdetect is not installed. This change makes langdetect required for test runs.

Although this is easy to resolve with poetry install --all-extras, the code intends to work without that; it's a worthwhile fix.

To Do

• Documentation.
• Changelog.
• Tests.

What changed?

• Added langdetect to the required test dependencies.
• Ran poetry lock to handle this new requirement.

How tested?

Ran locally.

• Tests now execute on my machine.

[github-actions]

Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry.

[Copilot]

Pull Request Overview

This PR ensures local tests don’t fail when langdetect isn’t installed by deferring the lyrics plugin import until after a skip guard.

• Moved the from beetsplug import lyrics import below the langdetect availability check.
• Skips the entire test module early if langdetect is missing outside of CI.

Comments suppressed due to low confidence (2)

test/plugins/test_lyrics.py:30

• The skip guard is placed after importing .lyrics_pages, which may also depend on langdetect. Move the skip check above any imports that could import code requiring langdetect to avoid import errors.

from .lyrics_pages import LyricsPage, lyrics_pages

test/plugins/test_lyrics.py:33

• [nitpick] Consider using pytest.importorskip('langdetect', reason='langdetect is required for lyrics tests') instead of a manual skip guard for clearer intent and simpler code.

if not github_ci and not importlib.util.find_spec("langdetect"):

[semohr]

There are some failing tests on python 3.9 that needs a bit of investigation. I don't see how this is related to the changes here tho.

@wisp3rwind & @snejus Do you know where this could stem from? I think we had some issue with the tests for the lyrics plugin before.

[snejus]

Just had a look at the failing build. The integration test that checks chartlyrics.com is expected to fail: I discovered that this website is gone and removed the test under #5800.

If you want, you can cherry-pick this commit.

lyricsontop failure is unexpected - maybe it's just a one-off issue, so I've tried re-running the build.

Edit: lyricsontop issue is gone after the re-run.

[snejus]

Also note we'll want to add a small changelog note since this has been reported as an issue.

[citelao]

I've made langdetect required for tests, as recommended. However, regenerating the lockfile updated some other unrelated versions. It seems like we can get away with changing just the langdetect section of the lockfile, but I'm not confident in that change.

I currently recommend taking the full lockfile change.

Thoughts?

[snejus]

Try reverting the change in the lockfile and running poetry lock instead!

[citelao]

Reran with poetry lock --no-update.

[citelao]

Since I'm on Windows, the line endings might be changing. If y'all want to push a change with a smaller lockfile diff, I'm game.

[snejus]

@citelao could you try running sed -iz 's/\r\n/\n/g' poetry.lock and let me know if this fixes the line endings?

[citelao]

I did what had to be done 😛. I ran git config --global core.autocrlf false, reran poetry lock --no-update, and switched the line endings back to LF in VS Code. Should be fixed!

[snejus]

I did what had to be done 😛. I ran git config --global core.autocrlf false, reran poetry lock --no-update, and switched the line endings back to LF in VS Code. Should be fixed!

Thank you!

This issue will inevitably pop up again once we've got another contributor updating deps on Windows, so I'm looking for a fix-it-once-and-forever solution which does not depend on the developer editor setup. This is probably going to be a poe hook.

While you here @citelao could you by any chance help me to test it? I am not using a Windows machine so it's a bit complicated 😅

1. Update pyproject.toml

diff --git a/pyproject.toml b/pyproject.toml
index ea69240d5..ee82cf6e0 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -154,14 +154,27 @@ web = ["flask", "flask-cors"]
 [tool.poetry.scripts]
 beet = "beets.ui:main"
 
+[tool.poetry.requires-plugins]
+poethepoet = ">=0.32"
+
 [build-system]
 requires = ["poetry-core>=1.0.0"]
 build-backend = "poetry.core.masonry.api"
 
 [tool.pipx-install]
-poethepoet = ">=0.26"
+poethepoet = ">=0.32"
 poetry = ">=1.8,<2"
 
+[tool.poe.poetry_hooks]
+post_add = "fix-line-endings poetry.lock"
+post_lock = "fix-line-endings poetry.lock"
+post_remove = "fix-line-endings poetry.lock"
+post_update = "fix-line-endings poetry.lock"
+
+[tool.poe.tasks.fix-line-endings]
+args = { file = { help = "File to fix line endings for", positional = true, required = true } }
+shell = "sed 's/\r//g' poetry.lock -i.bak && rm poetry.lock.bak"
+
 [tool.poe.tasks.build]
 help = "Build the package"
 shell = """

2. Run poetry install to make sure that the poethepoet plugin is installed.
3. Try, say, poetry update

Are line endings looking as they should?

Note: I was going to push to your branch but found I don't have permissions to push to your fork

[citelao]

I guarantee you that won't work :) Windows machines don't have sed (some folks install msys2 or mingw, but that's not guaranteed). You'll want a cross-platform way of fixing line endings.

Quick investigation seems to point to a poetry issue: Preserve line breaks in poetry.lock (regression) (and a fix as of Aug 18, 2024). New versions of poetry should maintain line endings when running poetry lock.

The issue thread did suggest a .gitattributes file as an alternative (e.g something like poetry.lock text eol=lf).

But I don't really understand the issue; my git supposedly normalized line endings for me, and I had to turn it off and then normalize manually to fix the issue.

[arogl]

I guarantee you that won't work :) Windows machines don't have sed (some folks install msys2 or mingw, but that's not guaranteed). You'll want a cross-platform way of fixing line endings.

sed is available if you installed git.

I should be able to get back to more windows work this weekend.

[citelao]

I guarantee you that won't work :) Windows machines don't have sed (some folks install msys2 or mingw, but that's not guaranteed). You'll want a cross-platform way of fixing line endings.

sed is available if you installed git.

I should be able to get back to more windows work this weekend.

You're right---looks like git bash has it (it's a mingw64 shell); I missed that. I'm used to working from plain PowerShell, though.

I think the problem will go away if we can update the poetry version? Or create a .gitattributes file?

[snejus]

I assumed you were using one of the supported shells

Wow, plain PowerShell, that's proper hardcore!

[snejus]

Well, in any case - we've got this fixed for now, so we're good.

[snejus]

If you rebase / merge recently updated master branch, the lyrics test issue should be gone.

[citelao]

Rebased!

[snejus]

Thanks for your help and patience!

[citelao]

Happy to help from my sed-less PowerShell :)