-
Notifications
You must be signed in to change notification settings - Fork 200
Improvements in tests process #297
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
Improvements in tests process #297
Conversation
|
Which version of Plex are you currently using for tests? There is no setting named |
|
Dunno, Its @pkkid test server. |
|
I dont know. @pkkid what version is the test server? Our test account don’t have plex pass so these need to be be skipped if plex pass is missing. |
|
Yep, I've just added a |
|
Dunno, my suggestion would be to build a root dir with tvshow, movies, photo and music. Add the files there and just zip them. We could download and setup the entire thing on demand. I’m a bit worried about testing time and I don’t want to keep downloading the same files over and over. Metadata creation might take some time too. |
|
Testing time is ~8 minutes (or 20 minutes, if you test all python versions sequentially) with bootstrapping from scratch: download the required media, set up libraries, wait for metadata, and this is exactly how the tests working in this PR. It's about 4 times longer the the your current approach. |
|
Ok, looks like I've resolved the issue with photos: Plex have 2 types of photo albums:
And I initially found only first one. Thank you for the attention :) I hope other issues would be less problematic. |
|
@andrey-yantsen - I'm OK with tests taking 4 times longer if we are standing up and tearing down the Plex server as part of it. It's a much more reliable way to test moving forward. -- We should add the requirements to get tests working to the README of the project. I think all it requires beyond the pip install is the environment variables mentioned above? |
This reverts commit 0d536bd.
|
Ok guys, seems like I've done here, please review all of this. As for websocket-client — version 0.53.0 didn't worked on Travis from the first try and I didn't investigated it further. |
tests/test_myplex.py
Outdated
| def test_myplex_webhooks(account): | ||
| if account.subscriptionActive: | ||
| assert not account.webhooks() | ||
| assert type(account.webhooks()) is list |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not isinstance?
plexapi/server.py
Outdated
| releases = self.fetchItems('/updater/status') | ||
| if len(releases): | ||
| return releases[0] | ||
| else: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the else part could just be removed. since returns None anyway.
| def seasons(self, **kwargs): | ||
| """ Returns a list of :class:`~plexapi.video.Season` objects. """ | ||
| key = '/library/metadata/%s/children' % self.ratingKey | ||
| key = '/library/metadata/%s/children?excludeAllLeaves=1' % self.ratingKey |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whats the benefit by excludeAllLeaves? Faster?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without it we'd have "All Seasons" as one more seasons, I think it's a bit unexpected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Im not sure i understand. This is supposed to returns all seasons. does that still happen?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, sure. If the show has 2 seasons it'll return [<Season 1>, <Season 2>], but if you'd omit excludeAllLeaves you'll receive [<All Seasons>, <Season 1>, <Season 2>].
|
Oh, guys, just remembered: I know nothing about client-related tests, can you please cover it in the docs? |
|
i dont think the clients is tested at all atm as it needs to be on the same local network as the server. Really stellar work btw! Thank you so much. |
|
Ooooh, I have one more idea, I can't stop it, sorry :) I'll try to automate the client tests this evening using Plex Web and some Selenium magic. |
|
I've made some progress with client tests, but in order to get them working I need to dig into PlexClient and rewrite some parts of it. You can take a look at my working branch for this, if you're interested: https://github.com/andrey-yantsen/python-plexapi/tree/client-tests. Shortly: In my working branch I was able to create an instance of Plex Web within a docker, and also I was able to play a photo/video on it using my real browser, but that's all what I've achieved. I'll perform another attempt later, but I think it would be better to do it within another PR. |
|
@Hellowlol you need to remove |

Currently I can see following problems with tests:
PKKidMyPlexAccounttest require that you don't have a PlexPass (and it's not really documented)As for now I've partially resolved first few easy issues, and stuck with generating a proper library: I have no idea how to get tests related to
photoalbumto pass, and I need your help here.To get my new approach working you have to define
PLEXAPI_AUTH_MYPLEX_USERNAMEandPLEXAPI_AUTH_MYPLEX_PASSWORDin the your Travis CI ENV variables, instead ofPLEXAPI_AUTH_SERVER_BASEURLandPLEXAPI_AUTH_SERVER_TOKEN.After each run of travis the newly created server and client would be removed from the MyPlex account.
You can check the Travis CI output by visiting https://travis-ci.org/andrey-yantsen/python-plexapi/builds/426206148.