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

Fix sync tests when using a token #620

Merged
merged 12 commits into from
Dec 9, 2020

Conversation

JonnyWong16
Copy link
Collaborator

@JonnyWong16 JonnyWong16 commented Dec 7, 2020

Description

Sync tests could fail because of conflicting parallel CI tests using the same token. When a token is created it is associated with a specific X-Plex-Client-Identifier. Using the token with different X-Plex- headers will update the device attributes at https://plex.tv/devices.xml except for the clientIdentifier. This causes issues because one CI could be running main tests which sets the header X-Plex-Provides=controller while another parallel CI running sync test sets the header X-Plex-Provides=sync-target. Depending on which API requests are being made at the time the sync tests could fail because it will suddenly be missing sync-target for the device.

This PR:

  • Updates MyPlexPinLogin to use https://plex.tv/api/v2 endpoint.
  • Adds helper function createMyPlexDevice that uses MyPlexPinLogin to create a new device on the Plex account.
  • Updates sync tests to create a separate sync device with a unique clientIdentifier. This gives the sync tests it's own sync device which won't conflict with the device associated with the token.

See workflow run here: https://github.com/JonnyWong16/python-plexapi/actions/runs/404885213

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated the docstring for new or existing methods
  • I have added tests when applicable

@codecov-io
Copy link

codecov-io commented Dec 7, 2020

Codecov Report

Merging #620 (2cf0d7e) into master (f22fa56) will decrease coverage by 0.28%.
The diff coverage is 30.23%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #620      +/-   ##
==========================================
- Coverage   65.86%   65.58%   -0.29%     
==========================================
  Files          20       20              
  Lines        4137     4161      +24     
==========================================
+ Hits         2725     2729       +4     
- Misses       1412     1432      +20     
Impacted Files Coverage Δ
plexapi/myplex.py 29.52% <25.00%> (-0.50%) ⬇️
plexapi/base.py 83.89% <100.00%> (ø)
plexapi/library.py 78.82% <100.00%> (ø)
plexapi/utils.py 94.90% <100.00%> (ø)

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 f22fa56...2cf0d7e. Read the comment docs.

@Hellowlol
Copy link
Collaborator

Thanks! Should the test run a claimed server or am it missing something?

@jjlawren
Copy link
Collaborator

jjlawren commented Dec 9, 2020

Thanks! Should the test run a claimed server or am it missing something?

Yes, a claimed server with an account that also has a valid Plex Pass.

@Hellowlol Hellowlol merged commit d5701aa into pkkid:master Dec 9, 2020
jjlawren pushed a commit that referenced this pull request Dec 16, 2020
* Make test Plex Pass entitlements a subset

* Fix create MyPlexDevice race condition

* Rename to clientId to be consistent

* Move link method to MyPlexAccount
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.

4 participants