Skip to content

Commit ba3230a

Browse files
committed
fix(cli): mute interactions on non tty sessions
Signed-off-by: Cory Rylan <crylan@nvidia.com>
1 parent 130279f commit ba3230a

4 files changed

Lines changed: 47 additions & 6 deletions

File tree

‎projects/cli/src/index.test.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,11 @@ describe('index', () => {
2525
expect(output).toContain('nve <cmd> [args]');
2626
});
2727

28+
it('should hide the banner when output is not interactive',()=>{
29+
expect(output).not.toContain('░██████████');
30+
expect(output).toContain('@nvidia-elements/cli');
31+
});
32+
2833
it('should provide api.list',()=>{
2934
expect(output).toContain('nve api.list [format]');
3035
});
@@ -163,7 +168,7 @@ describe('index', () => {
163168
it('should reject array arguments that exceed the schema limit',()=>{
164169
constresult=spawnSync(
165170
process.execPath,
166-
['dist/index.js','api.get','nve-card','nve-input','nve-button','nve-badge','nve-alert','nve-link'],
171+
['dist/index.js','api.get','nve-card','nve-input','nve-button','nve-badge'],
167172
{
168173
timeout: 10000,
169174
encoding: 'utf-8',
@@ -173,7 +178,7 @@ describe('index', () => {
173178
);
174179

175180
expect(result.status).toBe(1);
176-
expect(result.stderr).toContain('api.get accepts at most 5 names.');
181+
expect(result.stderr).toContain('api.get accepts at most 3 names.');
177182
});
178183
});
179184

‎projects/cli/src/index.ts‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,15 @@ import { hideBin } from 'yargs/helpers';
1111
import{performance}from'perf_hooks';
1212
import{typeManagedToolMethod,tools,ToolSupport,typeSchema}from'@internals/tools';
1313
import{installNve}from'./install.js';
14-
import{banner,colors,exitWithCompleteToolResult,exitWithToolError,getArgValue,runAsyncTool}from'./utils.js';
14+
import{
15+
banner,
16+
colors,
17+
exitWithCompleteToolResult,
18+
exitWithToolError,
19+
getArgValue,
20+
isInteractiveTerminal,
21+
runAsyncTool
22+
}from'./utils.js';
1523
import{notifyIfUpdateAvailable}from'./update.js';
1624

1725
exportconstVERSION='0.0.0';
@@ -76,7 +84,9 @@ yargsInstance.command(
7684
awaitexitWithToolError(result,message);
7785
}
7886
}else{
79-
constgreeting=colors.complete(`\x1b[?7l\n${JSON.parse(banner)}\n\n`);
87+
constgreeting=isInteractiveTerminal(process.stdout)
88+
? colors.complete(`\x1b[?7l\n${JSON.parse(banner)}\n\n`)
89+
: '';
8090
console.log(
8191
`${greeting}${colors.complete(`@nvidia-elements/cli (${BUILD_SHA})`)}\n\n${awaityargsInstance.getHelp()}`
8292
);

‎projects/cli/src/utils.test.ts‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,16 +38,24 @@ vi.mock('@inquirer/prompts', () => ({
3838
}));
3939

4040
describe('utils',()=>{
41+
conststderrIsTTYDescriptor=Object.getOwnPropertyDescriptor(process.stderr,'isTTY');
42+
4143
beforeEach(()=>{
4244
vi.clearAllMocks();
4345
vi.spyOn(console,'log').mockImplementation(()=>{});
4446
vi.spyOn(process,'exit').mockImplementation(()=>{
4547
thrownewError('process.exit called');
4648
});
49+
Object.defineProperty(process.stderr,'isTTY',{value: true,configurable: true});
4750
});
4851

4952
afterEach(()=>{
5053
vi.restoreAllMocks();
54+
if(stderrIsTTYDescriptor){
55+
Object.defineProperty(process.stderr,'isTTY',stderrIsTTYDescriptor);
56+
}else{
57+
deleteprocess.stderr.isTTY;
58+
}
5159
});
5260

5361
describe('constants',()=>{
@@ -93,7 +101,7 @@ describe('utils', () => {
93101

94102
it('should return different messages on multiple calls',()=>{
95103
constmessages=newSet();
96-
// Call multiple times to increase chance of getting different messages
104+
// Call repeatedly to increase the chance of getting different messages
97105
for(leti=0;i<10;i++){
98106
messages.add(getSpinnerProgressMessage());
99107
}
@@ -126,6 +134,16 @@ describe('utils', () => {
126134
deleteprocess.env.CI;
127135
});
128136

137+
it('should run function without spinner when stderr is not a TTY',async()=>{
138+
Object.defineProperty(process.stderr,'isTTY',{value: false,configurable: true});
139+
constargs={};
140+
constresult=awaitrunAsyncTool(args,mockFn);
141+
142+
expect(result).toBe('test result');
143+
expect(mockFn).toHaveBeenCalledWith(args);
144+
expect(ora).not.toHaveBeenCalled();
145+
});
146+
129147
it('should run function without spinner when start flag is true',async()=>{
130148
constargs={start: true};
131149
constresult=awaitrunAsyncTool(args,mockFn);

‎projects/cli/src/utils.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,20 @@ export function getSpinnerProgressMessage() {
4444
returnmessages[Math.floor(Math.random()*messages.length)];
4545
}
4646

47+
exportfunctionisInteractiveTerminal(stream: NodeJS.WriteStream){
48+
returnBoolean(stream.isTTY)&&!process.env.CI;
49+
}
50+
51+
functionshouldShowInteractiveProgress(args: Record<string,unknown>,options: RunAsyncToolOptions){
52+
return(options.interactiveProgress??true)&&isInteractiveTerminal(process.stderr)&&!args.start&&!args.log;
53+
}
54+
4755
exportasyncfunctionrunAsyncTool(
4856
args: Record<string,unknown>,
4957
fn: ManagedToolMethod<unknown>,
5058
options: RunAsyncToolOptions={}
5159
){
52-
constisInteractive=(options.interactiveProgress??true)&&!args.start&&!args.log&&!process.env.CI;
60+
constisInteractive=shouldShowInteractiveProgress(args,options);
5361
letspinner: Ora|undefined;
5462

5563
conststartTime=Date.now();

0 commit comments

Comments
 (0)