Skip to content

fix: flaky compiler test #1835

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.

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

Merged
merged 1 commit into from
Jan 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
27 changes: 26 additions & 1 deletion arduino-ide-extension/src/node/config-service-impl.ts
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
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
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
}
}