search

gnaudio / jabra-node-sdk

Jabra Node.Js SDK based on N-API
https://www.npmjs.com/package/@gnaudio/jabra-node-sdk
MIT License
0 stars 0 forks source link

Refactor misc #72

Closed dlaezza-jabra closed 3 years ago

dlaezza-jabra commented 3 years ago

I have been working on this for a while. I started long ago this summer, stopped for months, and today I saw I was almost there, so I finished.

Shortly, I turned the miscellanea manual test (src/manualtest/misc.ts) in an interactive CLI app. I grouped the API calls present there by logical feature, and present them as items in an interactive menu.

As you run the file, it will present you with a first choice, which is if you want to execute any non-device operation (like getting the SDK version). Then, it will prompt for choosing one group of device operations (like, equalizer). For every connected device, it will execute the operation once.

The groups are organized in an array, where every item has a description to be displayed in the menu and a callback to be executed if the group is selected. When prompting, the description are displayed along their index number in the array, which is used for selection.

It's honestly much easier to test this way, where the (alas) manual tests that we write at least are permanent, organized, and can be easily re-executed when we need to.

tfroehlich-jabra commented 3 years ago

Looks nice! We should consider something similar for SDK3.

martin-jabra commented 3 years ago

It may be intended but it doesn't look right that the test exits with an error when you execute the reboot test, presumably because the device list is cleared when they detach.

npm ERR! code ELIFECYCLE
npm ERR! errno 3221225477

The test should maybe catch this or recover in a way that makes it obvious that it is working as intended. Besides this minor thing, it looks good to me.

dlaezza-jabra commented 3 years ago

It may be intended but it doesn't look right that the test exits with an error when you execute the reboot test, presumably because the device list is cleared when they detach.

npm ERR! code ELIFECYCLE
npm ERR! errno 3221225477

The test should maybe catch this or recover in a way that makes it obvious that it is working as intended. Besides this minor thing, it looks good to me.

I just refactored the whole thing, I never really ran it. It might be that something got lost in the refactoring. We should keep it as an issue somewhere, or remove the test altogether.