Skip to content
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

Move a few more tests out of electron-browser #178305

Merged
merged 2 commits into from Mar 25, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andreamah @roblourens fyi this test was skipped so I decided to remove it. if you feel strong about it, then please look into restoring it on a layer that is not electron-browser which will eventually disappear. See #171508 for context.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was skipped because it was a "run on demand" test. I haven't looked at it in a long time, up to you @andreamah whether it's worth doing the work to keep. We haven't done any work in a long time that impacts backend search perf, you might just consider bringing it back if you do that kind of work in the future.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to bring it back then maybe consider to turn it into a node.js only test without electron dependencies if possible 👍

This file was deleted.

Expand Up @@ -16,11 +16,12 @@ import { NullLogService } from 'vs/platform/log/common/log';
import { flakySuite, getRandomTestPath } from 'vs/base/test/node/testUtils';
import { DiskFileSystemProvider } from 'vs/platform/files/node/diskFileSystemProvider';
import { detectEncodingByBOM } from 'vs/workbench/services/textfile/test/node/encoding/encoding.test';
import { workbenchInstantiationService, TestNativeTextFileServiceWithEncodingOverrides } from 'vs/workbench/test/electron-browser/workbenchTestServices';
import { workbenchInstantiationService } from 'vs/workbench/test/electron-browser/workbenchTestServices';
import createSuite from 'vs/workbench/services/textfile/test/common/textFileService.io.test';
import { IWorkingCopyFileService, WorkingCopyFileService } from 'vs/workbench/services/workingCopy/common/workingCopyFileService';
import { WorkingCopyService } from 'vs/workbench/services/workingCopy/common/workingCopyService';
import { UriIdentityService } from 'vs/platform/uriIdentity/common/uriIdentityService';
import { TestNativeTextFileServiceWithEncodingOverrides } from 'vs/workbench/test/electron-sandbox/workbenchTestServices';

flakySuite('Files - NativeTextFileService i/o', function () {
const disposables = new DisposableStore();
Expand Down
Expand Up @@ -12,7 +12,7 @@ import { ServiceCollection } from 'vs/platform/instantiation/common/serviceColle
import { DisposableStore } from 'vs/base/common/lifecycle';
import { FileService } from 'vs/platform/files/common/fileService';
import { NullLogService } from 'vs/platform/log/common/log';
import { workbenchInstantiationService, TestNativeTextFileServiceWithEncodingOverrides, TestServiceAccessor } from 'vs/workbench/test/electron-browser/workbenchTestServices';
import { TestNativeTextFileServiceWithEncodingOverrides, TestServiceAccessor, workbenchInstantiationService } from 'vs/workbench/test/electron-sandbox/workbenchTestServices';
import { IWorkingCopyFileService, WorkingCopyFileService } from 'vs/workbench/services/workingCopy/common/workingCopyFileService';
import { WorkingCopyService } from 'vs/workbench/services/workingCopy/common/workingCopyService';
import { UriIdentityService } from 'vs/platform/uriIdentity/common/uriIdentityService';
Expand All @@ -28,7 +28,7 @@ suite('Files - NativeTextFileService', function () {
let instantiationService: IInstantiationService;

setup(() => {
instantiationService = workbenchInstantiationService(disposables);
instantiationService = workbenchInstantiationService(undefined, disposables);

const logService = new NullLogService();
const fileService = new FileService(logService);
Expand Down
Expand Up @@ -4,23 +4,23 @@
*--------------------------------------------------------------------------------------------*/

import { localize } from 'vs/nls';
import { Emitter } from 'vs/base/common/event';
import { Event, Emitter } from 'vs/base/common/event';
import { assertIsDefined } from 'vs/base/common/types';
import { Registry } from 'vs/platform/registry/common/platform';
import { IWorkbenchContributionsRegistry, Extensions as WorkbenchExtensions } from 'vs/workbench/common/contributions';
import { LifecyclePhase } from 'vs/workbench/services/lifecycle/common/lifecycle';
import { ILifecycleService, LifecyclePhase, WillShutdownEvent } from 'vs/workbench/services/lifecycle/common/lifecycle';
import { WorkingCopyHistoryTracker } from 'vs/workbench/services/workingCopy/common/workingCopyHistoryTracker';
import { Disposable } from 'vs/base/common/lifecycle';
import { IWorkingCopyHistoryEntry, IWorkingCopyHistoryEntryDescriptor, IWorkingCopyHistoryEvent, IWorkingCopyHistoryService, MAX_PARALLEL_HISTORY_IO_OPS } from 'vs/workbench/services/workingCopy/common/workingCopyHistory';
import { FileOperationError, FileOperationResult, IFileService, IFileStatWithMetadata } from 'vs/platform/files/common/files';
import { IRemoteAgentService } from 'vs/workbench/services/remote/common/remoteAgentService';
import { URI } from 'vs/base/common/uri';
import { DeferredPromise, Limiter } from 'vs/base/common/async';
import { DeferredPromise, Limiter, RunOnceScheduler } from 'vs/base/common/async';
import { dirname, extname, isEqual, joinPath } from 'vs/base/common/resources';
import { IWorkbenchEnvironmentService } from 'vs/workbench/services/environment/common/environmentService';
import { hash } from 'vs/base/common/hash';
import { indexOfPath, randomPath } from 'vs/base/common/extpath';
import { CancellationToken } from 'vs/base/common/cancellation';
import { CancellationToken, CancellationTokenSource } from 'vs/base/common/cancellation';
import { ResourceMap } from 'vs/base/common/map';
import { IUriIdentityService } from 'vs/platform/uriIdentity/common/uriIdentity';
import { ILabelService } from 'vs/platform/label/common/label';
Expand Down Expand Up @@ -786,5 +786,83 @@ export abstract class WorkingCopyHistoryService extends Disposable implements IW

}

export class NativeWorkingCopyHistoryService extends WorkingCopyHistoryService {

private static readonly STORE_ALL_INTERVAL = 5 * 60 * 1000; // 5min

private readonly isRemotelyStored = typeof this.environmentService.remoteAuthority === 'string';

private readonly storeAllCts = this._register(new CancellationTokenSource());
private readonly storeAllScheduler = this._register(new RunOnceScheduler(() => this.storeAll(this.storeAllCts.token), NativeWorkingCopyHistoryService.STORE_ALL_INTERVAL));

constructor(
@IFileService fileService: IFileService,
@IRemoteAgentService remoteAgentService: IRemoteAgentService,
@IWorkbenchEnvironmentService environmentService: IWorkbenchEnvironmentService,
@IUriIdentityService uriIdentityService: IUriIdentityService,
@ILabelService labelService: ILabelService,
@ILifecycleService private readonly lifecycleService: ILifecycleService,
@ILogService logService: ILogService,
@IConfigurationService configurationService: IConfigurationService
) {
super(fileService, remoteAgentService, environmentService, uriIdentityService, labelService, logService, configurationService);

this.registerListeners();
}

private registerListeners(): void {
if (!this.isRemotelyStored) {

// Local: persist all on shutdown
this.lifecycleService.onWillShutdown(e => this.onWillShutdown(e));

// Local: schedule persist on change
this._register(Event.any(this.onDidAddEntry, this.onDidChangeEntry, this.onDidReplaceEntry, this.onDidRemoveEntry)(() => this.onDidChangeModels()));
}
}

protected getModelOptions(): IWorkingCopyHistoryModelOptions {
return { flushOnChange: this.isRemotelyStored /* because the connection might drop anytime */ };
}

private onWillShutdown(e: WillShutdownEvent): void {

// Dispose the scheduler...
this.storeAllScheduler.dispose();
this.storeAllCts.dispose(true);

// ...because we now explicitly store all models
e.join(this.storeAll(e.token), { id: 'join.workingCopyHistory', label: localize('join.workingCopyHistory', "Saving local history") });
}

private onDidChangeModels(): void {
if (!this.storeAllScheduler.isScheduled()) {
this.storeAllScheduler.schedule();
}
}

private async storeAll(token: CancellationToken): Promise<void> {
const limiter = new Limiter(MAX_PARALLEL_HISTORY_IO_OPS);
const promises = [];

const models = Array.from(this.models.values());
for (const model of models) {
promises.push(limiter.queue(async () => {
if (token.isCancellationRequested) {
return;
}

try {
await model.store(token);
} catch (error) {
this.logService.trace(error);
}
}));
}

await Promise.all(promises);
}
}

// Register History Tracker
Registry.as<IWorkbenchContributionsRegistry>(WorkbenchExtensions.Workbench).registerWorkbenchContribution(WorkingCopyHistoryTracker, LifecyclePhase.Restored);