Skip to content

Commit

Permalink
fix: flaky compiler test
Browse files Browse the repository at this point in the history 
 - The gRPC core client provider requires an initialized config service when processing the error message received during the `InitRequest`
 - Additional logging in the config service.
 - The tests log into the console.

Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
  • Loading branch information
@kittaakos
Akos Kitta authored and kittaakos committed Jan 27, 2023
1 parent 658f117 commit f5621db
Show file tree
Hide file tree
Showing 3 changed files with 125 additions and 10 deletions.
27 changes: 26 additions & 1 deletion arduino-ide-extension/src/node/config-service-impl.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,10 @@ export class ConfigServiceImpl
}

private async initConfig(): Promise<void> {
this.logger.info('>>> Initializing CLI configuration...');
try {
const cliConfig = await this.loadCliConfig();
this.logger.info('Loaded the CLI configuration.');
this.cliConfig = cliConfig;
const [config] = await Promise.all([
this.mapCliConfigToAppConfig(this.cliConfig),
Expand All @@ -153,10 +155,16 @@ export class ConfigServiceImpl
}),
]);
this.config.config = config;
this.logger.info(
`Mapped the CLI configuration: ${JSON.stringify(this.config.config)}`
);
this.logger.info('Validating the CLI configuration...');
await this.validateCliConfig(this.cliConfig);
delete this.config.messages;
this.logger.info('The CLI config is valid.');
if (config) {
this.ready.resolve();
this.logger.info('<<< Initialized the CLI configuration.');
return;
}
} catch (err: unknown) {
Expand All @@ -173,18 +181,35 @@ export class ConfigServiceImpl
): Promise<DefaultCliConfig> {
const cliConfigFileUri = await this.getCliConfigFileUri();
const cliConfigPath = FileUri.fsPath(cliConfigFileUri);
this.logger.info(`Loading CLI configuration from ${cliConfigPath}...`);
try {
const content = await fs.readFile(cliConfigPath, {
encoding: 'utf8',
});
const model = (yaml.safeLoad(content) || {}) as DefaultCliConfig;
this.logger.info(`Loaded CLI configuration: ${JSON.stringify(model)}`);
if (model.directories.data && model.directories.user) {
this.logger.info(
"'directories.data' and 'directories.user' are set in the CLI configuration model."
);
return model;
}
// The CLI can run with partial (missing `port`, `directories`), the IDE2 cannot.
// We merge the default CLI config with the partial user's config.
this.logger.info(
"Loading fallback CLI configuration to get 'directories.data' and 'directories.user'"
);
const fallbackModel = await this.getFallbackCliConfig();
return deepmerge(fallbackModel, model) as DefaultCliConfig;
this.logger.info(
`Loaded fallback CLI configuration: ${JSON.stringify(fallbackModel)}`
);
const mergedModel = deepmerge(fallbackModel, model) as DefaultCliConfig;
this.logger.info(
`Merged CLI configuration with the fallback: ${JSON.stringify(
mergedModel
)}`
);
return mergedModel;
} catch (error) {
if (ErrnoException.isENOENT(error)) {
if (initializeIfAbsent) {
Expand Down
9 changes: 6 additions & 3 deletions arduino-ide-extension/src/node/core-client-provider.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,8 +254,8 @@ export class CoreClientProvider {
}
})
.on('error', reject)
.on('end', () => {
const error = this.evaluateErrorStatus(errors);
.on('end', async () => {
const error = await this.evaluateErrorStatus(errors);
if (error) {
reject(error);
return;
Expand All @@ -265,7 +265,10 @@ export class CoreClientProvider {
});
}

private evaluateErrorStatus(status: RpcStatus[]): Error | undefined {
private async evaluateErrorStatus(
status: RpcStatus[]
): Promise<Error | undefined> {
await this.configService.getConfiguration(); // to ensure the CLI config service has been initialized.
const { cliConfiguration } = this.configService;
if (!cliConfiguration) {
// If the CLI config is not available, do not even try to guess what went wrong.
Expand Down
99 changes: 93 additions & 6 deletions arduino-ide-extension/src/test/node/core-service-impl.slow-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ import {
import { bindContributionProvider } from '@theia/core/lib/common/contribution-provider';
import { Disposable } from '@theia/core/lib/common/disposable';
import { EnvVariablesServer as TheiaEnvVariablesServer } from '@theia/core/lib/common/env-variables';
import { ILogger } from '@theia/core/lib/common/logger';
import { ILogger, Loggable } from '@theia/core/lib/common/logger';
import { LogLevel } from '@theia/core/lib/common/logger-protocol';
import { isWindows } from '@theia/core/lib/common/os';
import { waitForEvent } from '@theia/core/lib/common/promise-util';
import { MockLogger } from '@theia/core/lib/common/test/mock-logger';
Expand Down Expand Up @@ -66,7 +67,7 @@ describe('core-service-impl', () => {
let toDispose: Disposable[];

before(() => {
BackendApplicationConfigProvider.set({ configDirName: 'testArduinoIDE' });
BackendApplicationConfigProvider.set({ configDirName: '.testArduinoIDE' });
});

beforeEach(async function () {
Expand Down Expand Up @@ -175,10 +176,11 @@ function createContainer(): Container {
);
bind(EnvVariablesServer).toSelf().inSingletonScope();
bind(TheiaEnvVariablesServer).toService(EnvVariablesServer);
bind(ArduinoDaemonImpl).toSelf().inSingletonScope();
bind(ArduinoDaemon).toService(ArduinoDaemonImpl);
bind(MockLogger).toSelf().inSingletonScope();
bind(ILogger).toService(MockLogger);
bind(SilentArduinoDaemon).toSelf().inSingletonScope();
bind(ArduinoDaemon).toService(SilentArduinoDaemon);
bind(ArduinoDaemonImpl).toService(SilentArduinoDaemon);
bind(ConsoleLogger).toSelf().inSingletonScope();
bind(ILogger).toService(ConsoleLogger);
bind(TestNotificationServiceServer).toSelf().inSingletonScope();
bind(NotificationServiceServer).toService(TestNotificationServiceServer);
bind(ConfigServiceImpl).toSelf().inSingletonScope();
Expand Down Expand Up @@ -312,3 +314,88 @@ class TestCommandRegistry extends CommandRegistry {
return undefined;
}
}

@injectable()
class ConsoleLogger extends MockLogger {
override log(
logLevel: number,
arg2: string | Loggable | Error,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
...params: any[]
): Promise<void> {
if (arg2 instanceof Error) {
return this.error(String(arg2), params);
}
switch (logLevel) {
case LogLevel.INFO:
return this.info(arg2, params);
case LogLevel.WARN:
return this.warn(arg2, params);
case LogLevel.TRACE:
return this.trace(arg2, params);
case LogLevel.ERROR:
return this.error(arg2, params);
case LogLevel.FATAL:
return this.fatal(arg2, params);
default:
return this.info(arg2, params);
}
}

// eslint-disable-next-line @typescript-eslint/no-explicit-any
override async info(arg: string | Loggable, ...params: any[]): Promise<void> {
if (params.length) {
console.info(arg, ...params);
} else {
console.info(arg);
}
}

override async trace(
arg: string | Loggable,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
...params: any[]
): Promise<void> {
if (params.length) {
console.trace(arg, ...params);
} else {
console.trace(arg);
}
}

// eslint-disable-next-line @typescript-eslint/no-explicit-any
override async warn(arg: string | Loggable, ...params: any[]): Promise<void> {
if (params.length) {
console.warn(arg, ...params);
} else {
console.warn(arg);
}
}

override async error(
arg: string | Loggable,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
...params: any[]
): Promise<void> {
if (params.length) {
console.error(arg, ...params);
} else {
console.error(arg);
}
}

override async fatal(
arg: string | Loggable,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
...params: any[]
): Promise<void> {
return this.error(arg, params);
}
}

@injectable()
class SilentArduinoDaemon extends ArduinoDaemonImpl {
protected override onData(): void {
// NOOP
}
}

0 comments on commit f5621db

Please sign in to comment.