Increase BufferHelper test coverage to 100% #2052
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
springuper
commented
Dec 19, 2018
| @@ -12,8 +12,8 @@ function createMockBuffer (buffered) { | |||
|
|
|||
| describe('BufferHelper', function () { | |||
| describe('isBuffered', function () { | |||
| // |////////|__________|////////////////| | |||
| // 0 0.5 1 2.0 | |||
| // |////////|________|////////////////| | |||
There was a problem hiding this comment.
there are 10 underscores between 0.5 ~ 1, but only 8 slashes between 0 ~ 0.5, i make them equal.
| } | ||
| ]); | ||
| } | ||
| }; | ||
|
|
||
| it('should return true if media.buffered throw error', function () { |
There was a problem hiding this comment.
move down to put error handling test cases together.
michaelcunningham19
approved these changes
Dec 19, 2018
johnBartos
approved these changes
Jan 22, 2019
vitalibozhko
added a commit
to vitalibozhko/hls.js
that referenced
this pull request
Jan 29, 2019
…live-vtt-cues-cleanup * commit 'dcb3db62e1be5e53c4f491b0eecc3d5eabf54608': (48 commits) always trim command output add commit to version when PR tabs => spaces handle 'netlifyPr' in set-package-version check if repo is shallow before fetching configure netlify for PR's Fix video-dev#2061 Upgrade tests to Chai BDD & optimize runner (video-dev#2037) Choose bitrate in Bps (video-dev#2091) Listen to media detached Convert eme-controller to TypeScript. Increase adts.js test coverage to 100% (video-dev#2053) Increase BufferHelper test coverage to 100% (video-dev#2052) test: (() => { ... }) => function () { ... } test: change description test: add mocked _bwEstimator test: equal => strictEqual docs: fix link docs: update 2 docs: update ... # Conflicts: # src/controller/buffer-controller.ts
johnBartos
added a commit
to jwplayer/hls.js
that referenced
this pull request
Feb 22, 2019
* isCodecSupportedInMp4 test audio codecs as audio Fixes video-dev#1824. * Do not show `undefined` * package: rm npm-run-all as it contains "event-stream" (and maybe other dormant exploits?). see "flatmap-stream" * rewrite package-lock: removed event-stream and all deps got not fixed versions instead of a hat * update lock-file generated with npm v6.4.1 * package deps: npm remove --save mversion jshint * Added OpenPlayerJS to the list of players that support HLS.js (video-dev#2023) * NIT: fix typos in readme 1. Fix spelling of interdependent and environment 2. Correct the agreement in the phrase: 'imported/required by each other.' * Revert video-dev#2004 (video-dev#2027) * Fix 0.11.1 Regressions (video-dev#2028) * Create sourceBuffers if the max of 2 tracks has been reached, regardless of buffer codec events received * Replace Object.values with [].slice.call * Remux according to initial PTS so that streams start at 0 (video-dev#2030) * Buffer streams starting at 0 initPTS * Use PTS to determine if segments are contiguous * Remove code setting startPosition to buffer start * Remove seek to buffered start logic * Simplify EOS check by using frag states, move logic into shared superclass (video-dev#2040) * Simplify EOS check by using frag states, move logic into shared superclass * Move shared seek logic into base, clear fragCurrent on seek out of buffered range * Revert PTS wrapping code which produces incorrect cue times (video-dev#2044) * deploy demo/docs to netlify, not gh-pages * update demo links in issue template * format commit as code because it is slightly more readable * remove accidental "/" in readme * update note to access specific version api docs * include tag in idShort * escape ` * Check if PTS has wrapped after applying PTS time offset (video-dev#2056) * Check if PTS has wrapped after applying PTS time offset * Fix bug accessing buffered track array * Intialize vttCCs map with cc 0 (video-dev#2058) * Intiialize vttCCs map with cc 0 * use the version from package.json when deploying and include this in the deployments readme * fix: abrFactor greater than in docs * Changes clone function used in webpack config to be the webpack recommended merging library. * prod: add bandwidthEstimate method * docs: add bandwidthEstimate * test: add autotest * Convert buffer-controller.js to TypeScript Thank you to Tom Jenkinson for acting as a reviewer for this work, including writing some suggestions via the GitHub PR github.com/video-dev/pull/2073. The commit history has been squashed as it was a fairly lengthly process to get everything working to support the conversion to TypeScript. Early in the history of this commit, we converted properties to be initialized to `null`. This was incorrect, as it changes the behaviour of code that uses `Object.keys` calls to check if an object has any keys on it. Before, they would be empty, but if the properties are set to null, this check is no longer valid and it changes the behaviour. This is something to look out for in the future of other conversions, as the values themselves are often times reset to the `null`, but begin as undefined. Added a common `types` folder for interfaces where the ownership of the type wasn't clear for the individual file. The default no-op logger which is the inferred usage did not have any arguments in it's method signature. By adding an argument, TypeScript is able to infer the correct interface. As well the Karma and Webpack configuration had to be updated to enable building TypeScript, and getting testing coverage information from Istanbul. Define lib "es2015" and "dom" to add HTMLMediaElement, SourceBuffer and Number.isFinite definitions to the TypeScript binary when it is doing type-checking. When attempting to lint the older version of `typescript-eslint-parser` would emit `no-undef` errors for TypeScript interfaces. Updating the package fixes this issue. Tom pointed out that we should not use global module declarations to extend default HTML types since users will eventually be importing *.d.ts files from the hls.js project and this would also extend their types. Due to this feedback switched to defining a module local type `ExtendedSourceBuffer`. Which instead extends with the `ended` prop. Event Handlers were set to be part of the private scope. While this has no effect now, it will be important to ensure other modules do not directly invoke the event handlers. Other methods that were prefixed with a `_` were also set into the private scope. Where possible, code was switched to early-exit to lower the cognitive load in conditional statements, an example of this was the doAppend method. Another cleanup statement was switching handling of error codes and the comments to be closer to the conditional to which they applied. Switched to using the Record<Key, Value> type to help support access via the index signature [key: T] syntax in the future. An issue that was fixed in this commit was that The controller previous exposed the local `pendingTracks` property through the event and did not set the buffer on `this.tracks` instead opting to set it on the individual track, which bubbled up to the pendingTracks object, whose reference was lost in the buffer-controller module. This commit changes this behaviour so that the controller publishes `tracks` from the public variable on the controller rather than the pendingTracks which are passed into createSourceBuffers. This also means that this fixes the `.buffer.buffered` being undefined on the demo page. Added unit tests to cover the new behaviour of throwing an error if createSourceBuffer is called without an attached media element. * test: review changes * docs: review fix * prod: add review fix * docs: update Co-Authored-By: Beraliv <beraliv.spb@gmail.com> * docs: update 2 * docs: fix link * test: equal => strictEqual * test: add mocked _bwEstimator * test: change description * test: (() => { ... }) => function () { ... } * Increase BufferHelper test coverage to 100% (video-dev#2052) * Increase adts.js test coverage to 100% (video-dev#2053) * Increase adts.js test coverage to 100% * Convert eme-controller to TypeScript. Added TypeScript types to the code. Handled media detached and removing the encrypted event listener. * Listen to media detached Clears the encrypted event listener from the HTMLMediaElement in controller, and releases the stored reference. * Choose bitrate in Bps (video-dev#2091) Simplify bandwidth calculation by making; change docs to correctly reference bandwidth in bits/s * Upgrade tests to Chai BDD & optimize runner (video-dev#2037) * Convert tests to Chai BDD * Invoke mocha with exit so that travis doesnt hang on func test completion * Fix video-dev#2061 * configure netlify for PR's this follows the same process that runs for canary releases * check if repo is shallow before fetching because on netlify it clones the whole thing * handle 'netlifyPr' in set-package-version * tabs => spaces * add commit to version when PR * always trim command output * Update Fragment and LevelKey to typescript. * Update usage of Fragment and LevelKey. * Don't pass prevFrag into EXT-X-MAP handling. * Add tests for parsing byte range as seperate call. * Only parse byte range if it exists. * Apply suggestions from code review Co-Authored-By: itsjamie <1956521+itsjamie@users.noreply.github.com> * Review changes. parseByteRange -> setByteRange - removed it returning the parsed value since its usage was like a setter. * Review comments pt.2 * Change tests to call setByteRange * Explicit handling of 'initSegment' conversion for bitwise operation. * Fix lint warnings in m3u8-parser.js in prep for TS conversion. (video-dev#2106) * fix console null reference in logger (in webworkers) (video-dev#2095) Fix null reference in logger when console is not available * (misc) Adding ace editor to demo to enable advanced customization (video-dev#2077) * Add handling for AES-128 encrypted initialization segments needing IV. * Change IV handling for initSegments to just log a warning. * Update webpack to use babel loader with support for TS. (video-dev#2119) * Upgrade to webpack 4, babel 7 * Use straight git link for webworkify dep * Use simpler git url * Re-add globalObject workaround * Bump karma dep versions * Dont preprocess non-test files * Use @babel/register over ts-node * Update demo `const` usage. * Use webpack debug config in Karma configuration rather than duplication. * Delete duplicate declaration
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 PR will...
Add more tests for BufferHelper.
Why is this Pull Request needed?
In order to increase test coverage, here are the results:
before:
after:
Are there any points in the code the reviewer needs to double check?
No source code change, but please check whether these tests are good enough. basically, when i write test cases of method A who calls method B internally, i will trust method B and only test logics of method A. IMO, that's what unit tests mean.
So i write lots of test cases for
bufferedInfo, only a few forbufferInfobecause its logic is simple.Resolves issues:
No.
Checklist