feat(FEC-10356): upgrade Shaka #112
Conversation
issue: getManifest was marked as api which could change any time. solution: add extra check to make sure it's not breaking our API, keeps the tests as is to help us to be aware of changes on this API
|
@Yuvalke please also update dependency in Kaltura player |
There was a problem hiding this comment.
please check changes around 608/708 tracks: https://github.com/google/shaka-player/blob/master/docs/tutorials/upgrade-v2-5.md#embedded-text-cea-608708-api-change
In general read the guide to see what applies to us.
test/src/dash-adapter.spec.js
Outdated
| @@ -1414,7 +1414,7 @@ describe('DashAdapter: request filter', () => { | |||
| dashInstance.load(); | |||
| }); | |||
|
|
|||
| it('should apply promise filter for manifest', done => { | |||
| it.skip('should apply promise filter for manifest', done => { | |||
There was a problem hiding this comment.
can we just stub fetch or xhr themselves instead of shaka's?
I see it as we have a couple of options -
just get the entire fetch plugin, inherit from it and wrap the call to fetch
set spy on fetch and check the call url.
There was a problem hiding this comment.
@OrenMe I tried to stub fetch and xhr and it doesn't work.. the stub callback doesn't call on window.fetch/xhr.
inherit option will not work, I have tried to override their implementation...
they're using their own implementation
There was a problem hiding this comment.
Maybe override the function instead of stub?
There was a problem hiding this comment.
https://stackoverflow.com/questions/45946284/mock-http-fetch-in-sinon
does that help maybe?
There was a problem hiding this comment.
@RoyBregman override which function?
There was a problem hiding this comment.
There was a problem hiding this comment.
it seems that we can use the parse api call on those plugins. It is public and documented here for usage: https://github.com/google/shaka-player/blob/master/docs/tutorials/upgrade-v2-5.md#built-in-network-scheme-plugin-changes
There was a problem hiding this comment.
@OrenMe The stub on parse doesn't work since it does not exist in compile mode.
There was a problem hiding this comment.
ok, let's remove for now and open a task to rethink how to test correctly.
Description of the Changes
upgrade shaka to current latest, note this is a major change with breaking changes(see below).
issue: getManifest was marked as API which could change any time.
solution: add an extra check to make sure it's not breaking our API, keeps the tests as is to help us to be aware of changes on this API.
Also added a comment on google/shaka-player#
Solve: FEC-10356.
CheckLists