The bpd test bind a socket in order to test the protocol implementation. When
running concurrently this often resulted in an attempt to bind an already
occupied port.
By using the port number `0` we instead let the OS choose a free port. We then
have to extract it from the socket (which is handled by `bluelet`) via
`mock.patch`ing.
Return a namedtuple CommandOutput(stdout, stderr) instead of just stdout from
util.command_ouput, allowing separate access to stdout and stderr.
This change is required by the ffmpeg replaygain backend (GitHub
PullRequest #3056) as ffmpeg's ebur128 filter outputs only to stderr.
*All* URLs were checked manually, but only once per domain!
I mostly concerned myself with URLs in documentation rather than source
code because the latter may or may not have impactful changes, while the
former should be straight forward.
Changes in addition to simply adding an s:
- changed pip and pypi references as their location has changed
- MPoD (iOS app) url redirects to Regelian, so I replaced those
- updated homebrew references
Notable observations:
- beets.io does have HTTPS set up properly (via gh-pages)
- beatport.py uses the old HTTP url for beatport
- as does lyrics.py for lyrics.wikia.com
- https://tomahawk-player.org/ expired long ago, but the http page
redirects to https regardless
- none of the sourceforge subdomains have https (in 2019!)
Prevents reloading a model from the database when it hasn't changed.
Now we're back to almost the same speed as before the addition of album
field fallbacks.
The real MPD ignores `noidle` when the client is not idle. It doesn't
even send a successful response, just ignores the command. Although
I don't understand why a client would fail to keep track of its own
state, it seems that this is necessary to get ncmpcpp working.
The playlistid command is supposed to list the whole playlist if no
argument is provided, but we were accidentally trying to look up an
impossible negative id in that case causing an error to always be
returned.
When using pytest's test collector, any class with a name starting with
Test is collected. If it notices that the class has an `__init__` member
then it skips it with a warning since it's probably a false positive.
This isn't a big deal, but we can avoid warnings like this:
test/test_ui_importer.py:33
beets/test/test_ui_importer.py:33: PytestCollectionWarning: cannot collect test class 'TestTerminalImportSession' because it has a __init__ constructor
class TestTerminalImportSession(TerminalImportSession):
simply by renaming TestX to XFixture.
By comparing `sys.path` as setup by nose vs. testall.py it seems that we
weren't adding the top-level beets directory to the path. The script was
also previously changing the working directory before running the tests.
This is a hacky but effective way to work around a problem when running
the tests in parallel where two different test executions want to use
the same port.
This uses GStreamer APIs to extract a list of audio decoders and the
relevant MIME types and file extensions. Some clients like ncmpcpp use
this command to fetch a list of supported file extensions.
Some clients list the albums belonging to an artist by issuing the
command `list album <ARTIST NAME>`. This change inserts the tag `artist`
before the artist name so that this succeeds. Fixes#3007
A new `ControlConnection` is created each time a client connects over
a new control socket. This is used to forward events from the player,
and also for debugging utilities that are not part of the real MPD
protocol.
This new feature reuses as much infrastructure from the normal protocol
handling as possible (e.g. `Command` for parsing messages). While the
normal connection delegates to server `cmd_*` methods which are string
generators, the control connections delegate to `ctrl_*` methods defined
on the connection itself that are full coroutines.
Keep track of a list of currently-connected clients.
Use `socket.getpeername()` to get an identifier for each connection and
include this in each log message. This function is documented as not
being available on all systems, but it's unclear which systems this
involves.
Also log a message on client connect and disconnect events. If the
disconnection reason is because the client sent a blank line, match MPD
by returning a protocol error then hanging up. Escape curly braces.
Check function signature instead of using TypeError to crudely guess
that the wrong number of arguments were provided.
Prevent bpd from crashing when trying to log a traceback. The
`traceback.format_exc` function takes an optional argument which is
supposed to be an integer restricting the length of the backtrace to
show. Instead we were passing the exception object to this function and
causing a new exception to be raised.
Previously issuing the 'previous' command when at position 0 on the
playlist would cause bpd to stop playing. MPD instead just restarts the
currently playing song instead, so we now match this behaviour.
The songs are indexed starting from zero for the play command, however
the bound check was off by one. An index matching the length of the
playlist would crash the server instead of responding with an error
message over the protocol.
The repeat flag indicates that the entire playlist should be repeated.
If both the repeat and single flags are set then this triggers the old
behaviour of looping over a single track.
This command instructs bpd to stop playing when the current song
finishes. In the MPD 0.20 protocol this flag gains a value 'oneshot' but
for now we just support its older version with a boolean value.
The real MPD offers persistent playlist manipulation, storing the
playlists in a directory set in the config file. If that directory is
not available then the feature is disabled and the relevant commands all
respond with errors. Based on this, the initial support in bpd just
returns errors matching the MPD server in the disabled mode.
For playlistadd, extend the _bpd_add helper to work with playlists other
than the queue in order to support testing the real implementations of
these commands in the future.
There's a special status command for checking the replay gain mode,
which can be set to one of a short list of possible values. For now at
least we can ignore this feature, but track the setting anyway.
MPD supports a deprecated command 'volume' which was used to change the
volume by a relative amount unlike its replacement 'setvol' which uses
an absolute amount. As far as I can tell 'volume' always responds with a system
error message "No mixer".
These are a more sophisticated version of crossfade so we're free to
ignore them, at least for now. We now track the values of the two
settings, and show them in the status output. Like MPD, we suppress the
mixrampdb value if it's set to nan, which is how it signals that the
feature should be turned off.
If an MPC client is expecting a command to take an argument that bpd
isn't expecting (e.g. because of a difference in protocol versions) then
bpd currently crashes completely. Instead, do what the real MPD does and
return an error message over the protocol.
Although crossfade is not implemented in bpd, we can store the setting
and repeat is back to clients. Also log a warning that the operation is
not implemented.
The real MPD doesn't show the crossfade in status if it's zero since
that means no crossfade, so now we don't either.
The test `CommonOptionsParserCliTest.test_version` was passing with nose
but failing with pytest (see output below). The reason for the failure
seemed to be that the `test` plugin was loaded when it wasn't expected
to be loaded, changing the output of the `version` command. I'm not sur
exactly why that was happening, but since that test already inherited
from `TestHelper`, just invoking the plugin load/unload helper was
enough to fix it. I also removed the line setting the `self.lib`
variable since that's already done in the helper.
---
self = <test.test_ui.CommonOptionsParserCliTest testMethod=test_version>
def test_version(self):
l = self.run_with_output(u'version')
self.assertIn(u'Python version', l)
> self.assertIn(u'no plugins loaded', l)
E AssertionError: 'no plugins loaded' not found in 'beets version 1.4.8\nPython version 3.7.3rc1\nplugins: test\n'
test/test_ui.py:1292: AssertionError
Decode the bytes to strings: the MPD protocol specifies that the
communications are all in UTF-8.
Also parse the body into a dict since this is typically more convenient
than having to do it manually in each test.
The BOD tests are currently forking a process with a server running, and
this attempts to close stdin. Tests were failing due to DummyIn not
implementing the close() method. Adding this simple no-op does the trick
to allow forking and seems like a harmless addition.
The MPD protocol allows batching commands. There are two choices for the
first message that announced a batch is starting, and we just go for the
one that causes the server to respond with a marker message between each
individual response, since that's easier. This might need to be tweaked
in order to test the behaviour with the other batch indicator.
MPD now supports more fields ("tags") than what BPD advertises. Fixing
this should be a simple task of extending the mapping of fields to tags
in BPD's implementation.
The current MPD spec is several versions ahead of BPD. These tests just
compare the full specified list of commands against what BPD claims to
support (without actually testing their implementations). It's handy as
a measure of progress against the specification, but can perhaps be
dropped later once all of the individual protocol methods have tests.
Fix incorrect split of a tracklist by medium for the case of
two-sided mediums (#2887).
Following the discussion in #2887, the 'medium_total' value should
contain the number of tracks on the medium to which each particular
track belongs, not the total number of different mediums present on
a release.
Fix unit tests accordingly.
This is related to #2688 where a list of hard-coded non-audio formats to
ignore has been added. Some users may want to rip the audio portion of
video tracks (e.g. DVD-Video) so it would be beneficial to let them
control exactly which formats to ignore.
I added a `ignored_formats` setting for that purpose and moved the
hard-coded list into the config. Test and documentation have been
updated accordingly.
Aside: I also clarified the changelog a bit regarding this change and
the related one for #1210.
This ignores non-audio tracks during import:
- Data tracks, based on their title `[data track]` (which seems to be
the MusicBrainz convention, as there's no specific flag to indicate
that a track is a data one),
- Video tracks, based on the `video=true` attribute.
It's similar to the Picard changes mentioned in #1210, except it doesn't
deal with `[silence]` tracks: These ones will probably require a setting
to let the user control if they should be imported or not.
Some releases have non-audio media, such as CD+DVD or CD+DVD-Video. Skip
these media when fetching album info as they will never match audio
tracks and will always report missing tracks.
I took the naive approach of cherry-picking a list of media suspected to
not contain audio from the MusicBrainz formats list:
https://musicbrainz.org/doc/Release/Format
The regex «[\+-]?[0-9]*» possibly matches a single minus/plus, which would then
be passed on to int(), raising a ValueError from within _safe_cast. The test
suite covered this for float, but not for int.
We now make sure we actually have a number after the sign by using a Kleene
plus.
The move functions in library.py and manipule_files in importer.py where
changed to use a single parameter for the file operation instead of
multiple boolean flags.
A typo in the documentation of the Album.move and Item.move functions
confusing True and False when describing the store parameter was fixed
as well.
mtime == 0 is the "this Item contains newer metadata than the file's
tags" marker. Setting this to something else than 0 emulates the state
of Items freshly read from the database.
Breaks 4 of the edit plugin tests.
The old tests were wrong but the incorrectness was hidden by the incorrect
parameter passing fixed in the previous commit. Now we actually test that the
item's path did not change.
Turns out! The $ character in Python regexes also matches before the last
newline at the end of a string, not just at the end of a string. The \Z entity
does what we really want: the *real* end of the string.
Move existing tests into LyricsGooglePluginMachineryTest.
Create LyricsPluginSourcesTest class to check fetching of each source.
Some code was supposed to do that until now but was never executed as we
exited early at the "if not check_lyrics_fetched():" check.
