SwagLyrics / SwSpotify

Cross-platform library to get the currently playing song and artist from Spotify w/o using the API or the internet. Very fast.
MIT License
85 stars 12 forks source link

Try test fixing #8

Closed aadibajpai closed 4 years ago

pep8speaks commented 4 years ago

Hello @aadibajpai! Thanks for updating this PR.

There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:

Comment last updated at 2019-10-24 21:47:53 UTC
marioortizmanero commented 4 years ago

hah! I tried the same thing but I ran into errors because I didn't know you could do mock.side_effect = DBusException. Good job

codecov[bot] commented 4 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@a88528d). Click here to learn what that means. The diff coverage is 77.77%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master       #8   +/-   ##
=========================================
  Coverage          ?   68.05%           
=========================================
  Files             ?        2           
  Lines             ?       72           
  Branches          ?        8           
=========================================
  Hits              ?       49           
  Misses            ?       21           
  Partials          ?        2
Impacted Files Coverage Δ
SwSpotify/spotify.py 75.38% <77.77%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update a88528d...88c412a. Read the comment docs.

aadibajpai commented 4 years ago

windows tests fixed!

aadibajpai commented 4 years ago

@marioortizmanero any idea how to install dbus on travis? I've fixed it for windows but I'm not on linux other than wsl and import dbus just works there

marioortizmanero commented 4 years ago

I get errors on my machine when I run pytest too. But to install DBus you can follow my project's .travis.yml (although it also installs other stuff). The only dependencies are GLib and DBus. Anyway I think DBus is correctly installed, it's just that the tests are failing.

But as I said, these tests don't make sense to me. For what I see, it just raises a DBusException inside artist() and expects a SpotifyNotRunning. Which isn't the case in this implementation (?)

aadibajpai commented 4 years ago

I get errors on my machine when I run pytest too. But to install DBus you can follow my project's .travis.yml (although it also installs other stuff). The only dependencies are GLib and DBus. Anyway I think DBus is correctly installed, it's just that the tests are failing.

But as I said, these tests don't make sense to me. For what I see, it just raises a DBusException inside artist() and expects a SpotifyNotRunning. Which isn't the case in this implementation (?)

The artist function calls the current function which raises a SpotifyClosed when there's a dbus exception which should be caught by a SpotifyNotRunning since it's inherited from that.

marioortizmanero commented 4 years ago

No, since this update "when there's a dbus exception which should be caught by SpotifyNotRunning" is incorrect because catching the dbus exceptions isn't done for the all of the function. It's limited to the first part where you actually use the API.

The only part of the DBus API usage that doesn't catch exceptions is session_bus = dbus.SessionBus() because an error in this doesn't have to do with Spotify.

And the rest of the function doesn't even use the DBus API.

aadibajpai commented 4 years ago

Okay stuff works.