Enable commands to report progress #95
Conversation
| "listr": "^0.14.3", | ||
| "shelljs": "^0.8.3", | ||
| "enquirer": "^2.3.6", | ||
| "listr2": "^3.3.1", |
There was a problem hiding this comment.
I got rid of shelljs since we are not using it (a long time ago we were, but not anymore). Also instead of listr I'm going to use listr2 (and enquirer which listr2 needs).
listr is a nice library that we can use to print command progress but is seems to be dead, as it has not taken fixes/updates in a long time. listr2 is an effort by someone else to update listr and add more features to it.
In fact, there is a thread where the creator of listr and the creator of listr2 and few others are discussing the idea of merging all of these back into one library at some point.
But until then, we should use listr2 b/c it has the same MIT license, has all of the listr features, has many more bug fixes, and has more features that are useful to us (such as being able to use Promises as tasks).
| expect(createNewDeviceMock).toHaveBeenCalledWith( | ||
| deviceName, | ||
| iOSDeviceType, | ||
| iOSSupportedRuntimes[0] | ||
| ); | ||
| }); | ||
|
|
||
| test('Logger must be initialized and invoked', async () => { |
There was a problem hiding this comment.
There's no value in this test. create command extends setup and we already test this in setup.
| @@ -24,44 +23,31 @@ export interface Requirement { | |||
| logger: Logger; | |||
| } | |||
|
|
|||
| export interface SetupTestCase { | |||
There was a problem hiding this comment.
we can simplify things by merging testResult message supplementalMessage
| } | ||
|
|
||
| export interface RequirementList { | ||
| requirements: Requirement[]; | ||
| executeSetup(): Promise<SetupTestResult>; | ||
| } | ||
|
|
||
| export interface Launcher { |
There was a problem hiding this comment.
Launcher is not used anywhere
| launchNativeBrowser(url: string): Promise<void>; | ||
| } | ||
|
|
||
| export interface WrappedPromiseResult { |
There was a problem hiding this comment.
There is no need to have different result types and mapping between them.
| }; | ||
|
|
||
| let totalDuration = 0; | ||
| const setupTasks = new Listr( |
There was a problem hiding this comment.
create a Listr object for Setup which itself holds another Lister object for all of the subtasks (resulting in auto-indentation when printing output)
src/common/Requirements.ts
Outdated
| testResult.hasMetAllRequirements = false; | ||
| } | ||
|
|
||
| subTask.title = this.getFormattedTitle( |
There was a problem hiding this comment.
we'll start with requirement.title as the task title, but will update the title (with colored passed/failed messages) after a task is completed.
| @@ -20,7 +20,7 @@ export class CommonUtils { | |||
| return Promise.resolve(); | |||
| } | |||
|
|
|||
| public static delay(ms: number): Promise<void> { | |||
| public static async delay(ms: number): Promise<void> { | |||
There was a problem hiding this comment.
LOL I find this a little amusing after your last epic code clean-up. 😉 Nice catch! 😀
There was a problem hiding this comment.
yes, looks like I'm the one who's been contributing to the problem 😆
|
Closing this PR since most of this will now go into lwc-dev-mobile-core (see this PR) |
This PR enabled the Setup requirements to be executed in parallel and have their progress reported visually. It also adds progress report (i.e.
cli.action) to other commands.