-
Notifications
You must be signed in to change notification settings - Fork 132
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
tests: remove XHRMock util #1291
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
peaBerberian
added
work-in-progress
This Pull Request or issue is not finished yet
Priority: 3 (Low)
This issue or PR has a low priority.
labels
Sep 26, 2023
peaBerberian
force-pushed
the
next-v4
branch
2 times, most recently
from
September 27, 2023 08:30
b5e1161
to
7c01cf7
Compare
peaBerberian
force-pushed
the
misc/remove-xhrmock-in-tests
branch
from
September 27, 2023 08:51
215d7f9
to
67e66c9
Compare
peaBerberian
force-pushed
the
next-v4
branch
from
September 27, 2023 15:50
45516d8
to
43a7359
Compare
peaBerberian
force-pushed
the
misc/remove-xhrmock-in-tests
branch
2 times, most recently
from
September 27, 2023 16:47
76ff342
to
d0c38e4
Compare
peaBerberian
force-pushed
the
next-v4
branch
from
September 29, 2023 17:49
43a7359
to
fd7ef33
Compare
peaBerberian
force-pushed
the
misc/remove-xhrmock-in-tests
branch
from
September 29, 2023 17:50
d0c38e4
to
fc8c99c
Compare
peaBerberian
force-pushed
the
next-v4
branch
2 times, most recently
from
October 13, 2023 09:29
7ff2cd1
to
ed9d45f
Compare
peaBerberian
force-pushed
the
misc/remove-xhrmock-in-tests
branch
from
October 13, 2023 09:35
995037a
to
09c0757
Compare
peaBerberian
force-pushed
the
misc/remove-xhrmock-in-tests
branch
from
October 13, 2023 09:49
09c0757
to
10d633f
Compare
peaBerberian
force-pushed
the
next-v4
branch
2 times, most recently
from
October 26, 2023 11:54
c9dd85f
to
6bb040d
Compare
peaBerberian
force-pushed
the
misc/remove-xhrmock-in-tests
branch
2 times, most recently
from
October 26, 2023 13:34
e46fcea
to
54c1b65
Compare
peaBerberian
added a commit
that referenced
this pull request
Oct 26, 2023
…contents Thanks to a test rewrite linked to #1291, I noticed that the v4 broke the possibility of relying on a `segmentLoader` when relying on smooth streaming, due to forgetting to update some property name in a refactoring of the transport code. This commit/branch fixes just that.
peaBerberian
force-pushed
the
misc/remove-xhrmock-in-tests
branch
3 times, most recently
from
October 30, 2023 13:10
37dba63
to
9d35029
Compare
peaBerberian
changed the title
[WIP] tests: begin removing XHRMock util
tests: remove XHRMock util
Oct 30, 2023
peaBerberian
force-pushed
the
misc/remove-xhrmock-in-tests
branch
from
November 6, 2023 17:10
9d35029
to
7999d4d
Compare
This is a Work-In-Progress to remove the `XHRMock` utils from our tests, which monkey-patched the browser's `XMLHttpRequest` API to allow mocking requests, "locking" them, redirecting them and so on (kind of like sinon.js's [nise](https://github.com/sinonjs/nise) which was a major inspiration but with added locking/unlocking capabilities). The reasons for doing this are: - It only worked for `XMLHttpRequest` and not for the `fetch` browser API which we use to fetch low-latency segments for now (due to some API differences between the two) and plan to use for more things in the future. Adding support for `fetch` looks like huge work (we have to re-implement the most part of the API in JS - may be fun but there are funnier tasks which are also more useful :p) - We have RxPlayer API which allows an application to provide its own fetching logic (`manifestLoader` and `segmentLoader`) which covers almost all fetching cases (it does not cover XLink charging, low-latency segment fetching and future Content Steering Manifest fetching) and covers all fetching cases currently done by `XHRMock`. - The now probable incoming future in-Worker feature of the RxPlayer cannot easily rely on monkey-patching its API, at least in integration tests (well, in all honesty, we will have to think about how we'll be testing this feature). - It was complex, looked like an NIH-y `fakeServer` implementation. Removing it in favor or simpler solutions is just easier to understand. This is not completely done yet.
peaBerberian
force-pushed
the
misc/remove-xhrmock-in-tests
branch
from
November 6, 2023 17:12
7999d4d
to
023c4b2
Compare
peaBerberian
added a commit
that referenced
this pull request
Nov 7, 2023
…contents Thanks to a test rewrite linked to #1291, I noticed that the v4 broke the possibility of relying on a `segmentLoader` when relying on smooth streaming, due to forgetting to update some property name in a refactoring of the transport code. This commit/branch fixes just that.
peaBerberian
added a commit
that referenced
this pull request
Nov 13, 2023
…contents Thanks to a test rewrite linked to #1291, I noticed that the v4 broke the possibility of relying on a `segmentLoader` when relying on smooth streaming, due to forgetting to update some property name in a refactoring of the transport code. This commit/branch fixes just that.
peaBerberian
added a commit
that referenced
this pull request
Nov 14, 2023
…contents Thanks to a test rewrite linked to #1291, I noticed that the v4 broke the possibility of relying on a `segmentLoader` when relying on smooth streaming, due to forgetting to update some property name in a refactoring of the transport code. This commit/branch fixes just that.
peaBerberian
added a commit
that referenced
this pull request
Nov 14, 2023
…contents Thanks to a test rewrite linked to #1291, I noticed that the v4 broke the possibility of relying on a `segmentLoader` when relying on smooth streaming, due to forgetting to update some property name in a refactoring of the transport code. This commit/branch fixes just that.
peaBerberian
added a commit
that referenced
this pull request
Nov 23, 2023
…contents Thanks to a test rewrite linked to #1291, I noticed that the v4 broke the possibility of relying on a `segmentLoader` when relying on smooth streaming, due to forgetting to update some property name in a refactoring of the transport code. This commit/branch fixes just that.
peaBerberian
added a commit
that referenced
this pull request
Dec 4, 2023
…contents Thanks to a test rewrite linked to #1291, I noticed that the v4 broke the possibility of relying on a `segmentLoader` when relying on smooth streaming, due to forgetting to update some property name in a refactoring of the transport code. This commit/branch fixes just that.
peaBerberian
added a commit
that referenced
this pull request
Dec 5, 2023
…contents Thanks to a test rewrite linked to #1291, I noticed that the v4 broke the possibility of relying on a `segmentLoader` when relying on smooth streaming, due to forgetting to update some property name in a refactoring of the transport code. This commit/branch fixes just that.
peaBerberian
added a commit
that referenced
this pull request
Dec 5, 2023
…contents Thanks to a test rewrite linked to #1291, I noticed that the v4 broke the possibility of relying on a `segmentLoader` when relying on smooth streaming, due to forgetting to update some property name in a refactoring of the transport code. This commit/branch fixes just that.
peaBerberian
added a commit
that referenced
this pull request
Dec 7, 2023
…contents Thanks to a test rewrite linked to #1291, I noticed that the v4 broke the possibility of relying on a `segmentLoader` when relying on smooth streaming, due to forgetting to update some property name in a refactoring of the transport code. This commit/branch fixes just that.
peaBerberian
added a commit
that referenced
this pull request
Dec 19, 2023
…contents Thanks to a test rewrite linked to #1291, I noticed that the v4 broke the possibility of relying on a `segmentLoader` when relying on smooth streaming, due to forgetting to update some property name in a refactoring of the transport code. This commit/branch fixes just that.
peaBerberian
added a commit
that referenced
this pull request
Dec 20, 2023
…contents Thanks to a test rewrite linked to #1291, I noticed that the v4 broke the possibility of relying on a `segmentLoader` when relying on smooth streaming, due to forgetting to update some property name in a refactoring of the transport code. This commit/branch fixes just that.
peaBerberian
added a commit
that referenced
this pull request
Dec 22, 2023
…contents Thanks to a test rewrite linked to #1291, I noticed that the v4 broke the possibility of relying on a `segmentLoader` when relying on smooth streaming, due to forgetting to update some property name in a refactoring of the transport code. This commit/branch fixes just that.
peaBerberian
added a commit
that referenced
this pull request
Jan 3, 2024
…contents Thanks to a test rewrite linked to #1291, I noticed that the v4 broke the possibility of relying on a `segmentLoader` when relying on smooth streaming, due to forgetting to update some property name in a refactoring of the transport code. This commit/branch fixes just that.
peaBerberian
added a commit
that referenced
this pull request
Jan 3, 2024
…contents Thanks to a test rewrite linked to #1291, I noticed that the v4 broke the possibility of relying on a `segmentLoader` when relying on smooth streaming, due to forgetting to update some property name in a refactoring of the transport code. This commit/branch fixes just that.
peaBerberian
added a commit
that referenced
this pull request
Jan 11, 2024
…contents Thanks to a test rewrite linked to #1291, I noticed that the v4 broke the possibility of relying on a `segmentLoader` when relying on smooth streaming, due to forgetting to update some property name in a refactoring of the transport code. This commit/branch fixes just that.
peaBerberian
added a commit
that referenced
this pull request
Jan 11, 2024
…contents Thanks to a test rewrite linked to #1291, I noticed that the v4 broke the possibility of relying on a `segmentLoader` when relying on smooth streaming, due to forgetting to update some property name in a refactoring of the transport code. This commit/branch fixes just that.
peaBerberian
added a commit
that referenced
this pull request
Jan 15, 2024
…contents Thanks to a test rewrite linked to #1291, I noticed that the v4 broke the possibility of relying on a `segmentLoader` when relying on smooth streaming, due to forgetting to update some property name in a refactoring of the transport code. This commit/branch fixes just that.
peaBerberian
added a commit
that referenced
this pull request
Jan 23, 2024
…contents Thanks to a test rewrite linked to #1291, I noticed that the v4 broke the possibility of relying on a `segmentLoader` when relying on smooth streaming, due to forgetting to update some property name in a refactoring of the transport code. This commit/branch fixes just that.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Priority: 3 (Low)
This issue or PR has a low priority.
work-in-progress
This Pull Request or issue is not finished yet
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a Work-In-Progress to remove the
XHRMock
utils from our tests, which monkey-patched the browser'sXMLHttpRequest
API to allow mocking requests, "locking" them, redirecting them and so on (kind of like sinon.js's nise which was a major inspiration but with added locking/unlocking capabilities).The reasons for doing this are:
It only worked for
XMLHttpRequest
and not for thefetch
browser API which we use to fetch low-latency segments for now (due to some API differences between the two) and plan to use for more things in the future.Adding support for
fetch
looks like huge work (we have to re-implement the most part of the API in JS - may be fun but there are funnier tasks which are also more useful :p)We have RxPlayer API which allows an application to provide its own fetching logic (
manifestLoader
andsegmentLoader
) which covers almost all fetching cases (it does not cover DASH's XLink loading, low-latency segment fetching and future Content Steering Manifest fetching) and covers all fetching cases currently done byXHRMock
anyway.The now probable incoming future in-Worker feature of the RxPlayer cannot easily rely on monkey-patching native API it uses, at least in integration tests (well, in all honesty, we will have to think about how we'll be testing this).
It was complex, looked like an NIH-y
fakeServer
implementation.Removing it in favor or simpler solutions is just easier to understand.
This is not completely done yet.