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.

Sign up for GitHub

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

Jump to bottom

Migrate Node tests to TypeScript #958

Merged
merged 4 commits into from
Nov 21, 2022
Merged

Migrate Node tests to TypeScript #958

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
04bac11
Migrate Node tests to TypeScript
ibc Nov 20, 2022
1907844
Add NPM release:check task
ibc Nov 21, 2022
ff00a7b
CI node: use `npm ci` and do not test just in Linux
ibc Nov 21, 2022
32076d0
Try to make ESLint succeed in Windows
ibc Nov 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .gitattributes
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# Tell GitHub to ignore some folders when detecting the project language.
art/* linguist-vendored=true
doc/* linguist-vendored=true
node/test/* linguist-vendored=true
rust/benches/* linguist-vendored=true
rust/examples/* linguist-vendored=true
rust/examples-frontend/* linguist-vendored=true
Expand Down
3 changes: 1 addition & 2 deletions .github/workflows/mediasoup-node.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ jobs:
restore-keys: |
${{ matrix.ci.os }}-node-

- run: npm install
- run: npm ci
- run: npm run lint:node
if: runner.os == 'Linux'
- run: npm run test:node
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

## Others.
/coverage
/.cache
/NO_GIT
*.swp
*.swo
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

### NEXT

* Node: Migrate tests to TypeScript (PR #958).
* Node: Remove compiled JavaScript from repository and compile TypeScript code on NPM `prepare` script on demand when installed via git (PR #954).
* `Worker`: Add `RTC::Shared` singleton for RTC entities (PR #953).

Expand Down
11 changes: 8 additions & 3 deletions node/.eslintrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
const os = require('os');

const isWindows = os.platform() === 'win32';

const eslintConfig =
{
env :
Expand All @@ -15,7 +19,8 @@ const eslintConfig =
{
impliedStrict : true
},
lib : [ 'es2018' ]
lib : [ 'es2018' ],
project : 'node/tsconfig.json'
},
globals :
{
Expand Down Expand Up @@ -59,7 +64,7 @@ const eslintConfig =
}
],
'keyword-spacing' : 2,
'linebreak-style' : [ 2, 'unix' ],
'linebreak-style' : [ 2, isWindows ? 'windows' : 'unix' ],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would it depend on OS thought? This will mean people contributing from different operating systems having different code style that either passes Windows CI or *nix CI.

Copy link
Member Author

@ibc ibc Feb 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was for CI lint pass on Windows.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then it doesn't really make sense to me. Why would we want to check for different line breaks depending on OS check is running on?

Copy link
Member Author

@ibc ibc Feb 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty sure that line endings change in place from LF to CRLF when fetching the git repo on Windows. AFAIR that could be a feature of git. Probably it can be disabled.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I recall something like that too... Okay, thanks for clarifying.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here: https://stackoverflow.com/questions/21822650/disable-git-eol-conversions

AFAIU it's a local setting so we cannot force the Windows machine in GH CI to behave as we would wish.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, it is fine then, thanks!

'lines-around-comment' : [ 2,
{
allowBlockStart : true,
Expand Down Expand Up @@ -222,7 +227,7 @@ eslintConfig.overrides.push(

eslintConfig.overrides.push(
{
files : [ '*.js' ],
files : [ '*.ts' ],
env : {
...eslintConfig.env,
'jest/globals' : true
Expand Down
0 node/tests/data/dtls-cert.pem → node/src/tests/data/dtls-cert.pem
View file
Open in desktop
File renamed without changes.
0 node/tests/data/dtls-key.pem → node/src/tests/data/dtls-key.pem
View file
Open in desktop
File renamed without changes.
22 changes: 11 additions & 11 deletions node/tests/test-ActiveSpeakerObserver.js → node/src/tests/test-ActiveSpeakerObserver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
const { toBeType } = require('jest-tobetype');
const mediasoup = require('../lib/');
const { createWorker } = mediasoup;
import * as mediasoup from '../';

expect.extend({ toBeType });
const { createWorker } = mediasoup;

let worker;
let router;
let activeSpeakerObserver;
let worker: mediasoup.types.Worker;
let router: mediasoup.types.Router;
let activeSpeakerObserver: mediasoup.types.ActiveSpeakerObserver;

const mediaCodecs =
const mediaCodecs: mediasoup.types.RtpCodecCapability[] =
[
{
kind : 'audio',
Expand Down Expand Up @@ -41,7 +39,7 @@ test('router.createActiveSpeakerObserver() succeeds', async () =>

expect(onObserverNewRtpObserver).toHaveBeenCalledTimes(1);
expect(onObserverNewRtpObserver).toHaveBeenCalledWith(activeSpeakerObserver);
expect(activeSpeakerObserver.id).toBeType('string');
expect(typeof activeSpeakerObserver.id).toBe('string');
expect(activeSpeakerObserver.closed).toBe(false);
expect(activeSpeakerObserver.paused).toBe(false);
expect(activeSpeakerObserver.appData).toEqual({});
Expand All @@ -56,10 +54,12 @@ test('router.createActiveSpeakerObserver() succeeds', async () =>

test('router.createActiveSpeakerObserver() with wrong arguments rejects with TypeError', async () =>
{
// @ts-ignore
await expect(router.createActiveSpeakerObserver({ interval: false }))
.rejects
.toThrow(TypeError);

// @ts-ignore
await expect(router.createActiveSpeakerObserver({ appData: 'NOT-AN-OBJECT' }))
.rejects
.toThrow(TypeError);
Expand Down Expand Up @@ -102,7 +102,7 @@ test('ActiveSpeakerObserver emits "routerclose" if Router is closed', async () =
const router2 = await worker.createRouter({ mediaCodecs });
const activeSpeakerObserver2 = await router2.createAudioLevelObserver();

await new Promise((resolve) =>
await new Promise<void>((resolve) =>
{
activeSpeakerObserver2.on('routerclose', resolve);
router2.close();
Expand All @@ -113,7 +113,7 @@ test('ActiveSpeakerObserver emits "routerclose" if Router is closed', async () =

test('ActiveSpeakerObserver emits "routerclose" if Worker is closed', async () =>
{
await new Promise((resolve) =>
await new Promise<void>((resolve) =>
{
activeSpeakerObserver.on('routerclose', resolve);
worker.close();
Expand Down
22 changes: 11 additions & 11 deletions node/tests/test-AudioLevelObserver.js → node/src/tests/test-AudioLevelObserver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
const { toBeType } = require('jest-tobetype');
const mediasoup = require('../lib/');
import * as mediasoup from '../';
const { createWorker } = mediasoup;

expect.extend({ toBeType });
let worker: mediasoup.types.Worker;
let router: mediasoup.types.Router;
let audioLevelObserver: mediasoup.types.AudioLevelObserver;

let worker;
let router;
let audioLevelObserver;

const mediaCodecs =
const mediaCodecs: mediasoup.types.RtpCodecCapability[] =
[
{
kind : 'audio',
Expand Down Expand Up @@ -41,7 +38,7 @@ test('router.createAudioLevelObserver() succeeds', async () =>

expect(onObserverNewRtpObserver).toHaveBeenCalledTimes(1);
expect(onObserverNewRtpObserver).toHaveBeenCalledWith(audioLevelObserver);
expect(audioLevelObserver.id).toBeType('string');
expect(typeof audioLevelObserver.id).toBe('string');
expect(audioLevelObserver.closed).toBe(false);
expect(audioLevelObserver.paused).toBe(false);
expect(audioLevelObserver.appData).toEqual({});
Expand All @@ -64,14 +61,17 @@ test('router.createAudioLevelObserver() with wrong arguments rejects with TypeEr
.rejects
.toThrow(TypeError);

// @ts-ignore
await expect(router.createAudioLevelObserver({ threshold: 'foo' }))
.rejects
.toThrow(TypeError);

// @ts-ignore
await expect(router.createAudioLevelObserver({ interval: false }))
.rejects
.toThrow(TypeError);

// @ts-ignore
await expect(router.createAudioLevelObserver({ appData: 'NOT-AN-OBJECT' }))
.rejects
.toThrow(TypeError);
Expand Down Expand Up @@ -114,7 +114,7 @@ test('AudioLevelObserver emits "routerclose" if Router is closed', async () =>
const router2 = await worker.createRouter({ mediaCodecs });
const audioLevelObserver2 = await router2.createAudioLevelObserver();

await new Promise((resolve) =>
await new Promise<void>((resolve) =>
{
audioLevelObserver2.on('routerclose', resolve);
router2.close();
Expand All @@ -125,7 +125,7 @@ test('AudioLevelObserver emits "routerclose" if Router is closed', async () =>

test('AudioLevelObserver emits "routerclose" if Worker is closed', async () =>
{
await new Promise((resolve) =>
await new Promise<void>((resolve) =>
{
audioLevelObserver.on('routerclose', resolve);
worker.close();
Expand Down
Loading