Skip to content

Commit

Permalink
close transformer state dumps even if overrides threw an error (#3612)
Browse files Browse the repository at this point in the history
* close transformer state dumps even if producing them threw an error

Co-authored-by: Michael Belousov <[email protected]>
  • Loading branch information
MichaelBelousov and MichaelBelousov committed May 20, 2022
1 parent bf68f3a commit 9f8cea8
Show file tree
Hide file tree
Showing 15 changed files with 352 additions and 259 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@itwin/core-backend",
"comment": "",
"type": "none"
}
],
"packageName": "@itwin/core-backend"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@itwin/core-transformer",
"comment": "always close transformer resumption state db even on errors",
"type": "none"
}
],
"packageName": "@itwin/core-transformer"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@itwin/core-transformer",
"comment": "",
"type": "none"
}
],
"packageName": "@itwin/core-transformer"
}
4 changes: 3 additions & 1 deletion core/backend/src/test/HubMock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ export class HubMock {
return this._iTwinId;
}

protected static get knownTestLocations(): { outputDir: string, assetsDir: string } { return KnownTestLocations; }

/**
* Begin mocking IModelHub access. After this call, all access to IModelHub will be directed to a [[LocalHub]].
* @param mockName a unique name (e.g. "MyTest") for this HubMock to disambiguate tests when more than one is simultaneously active.
Expand All @@ -77,7 +79,7 @@ export class HubMock {
throw new Error("Either a previous test did not call HubMock.shutdown() properly, or more than one test is simultaneously attempting to use HubMock, which is not allowed");

this.hubs.clear();
this.mockRoot = join(KnownTestLocations.outputDir, "HubMock", mockName);
this.mockRoot = join(this.knownTestLocations.outputDir, "HubMock", mockName);
IModelJsFs.recursiveMkDirSync(this.mockRoot);
IModelJsFs.purgeDirSync(this.mockRoot);
try {
Expand Down
17 changes: 10 additions & 7 deletions core/backend/src/test/IModelTestUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,14 +91,15 @@ export enum TestUserType {
* All methods in this class should be usable with any BackendHubAccess implementation (i.e. HubMock and IModelHubBackend).
*/
export class HubWrappers {
protected static get hubMock() { return HubMock; }

public static async getAccessToken(user: TestUserType) {
return TestUserType[user];
}

/** Create an iModel with the name provided if it does not already exist. If it does exist, the iModelId is returned. */
public static async createIModel(accessToken: AccessToken, iTwinId: GuidString, iModelName: string): Promise<GuidString> {
assert.isTrue(HubMock.isValid, "Must use HubMock for tests that modify iModels");
assert.isTrue(this.hubMock.isValid, "Must use HubMock for tests that modify iModels");
let iModelId = await IModelHost.hubAccess.queryIModelByName({ accessToken, iTwinId, iModelName });
if (!iModelId)
iModelId = await IModelHost.hubAccess.createNewIModel({ accessToken, iTwinId, iModelName, description: `Description for iModel` });
Expand All @@ -109,7 +110,7 @@ export class HubWrappers {
* @returns the iModelId of the newly created iModel.
*/
public static async recreateIModel(...[arg]: Parameters<BackendHubAccess["createNewIModel"]>): Promise<GuidString> {
assert.isTrue(HubMock.isValid, "Must use HubMock for tests that modify iModels");
assert.isTrue(this.hubMock.isValid, "Must use HubMock for tests that modify iModels");
const deleteIModel = await IModelHost.hubAccess.queryIModelByName(arg);
if (undefined !== deleteIModel)
await IModelHost.hubAccess.deleteIModel({ accessToken: arg.accessToken, iTwinId: arg.iTwinId, iModelId: deleteIModel });
Expand Down Expand Up @@ -164,7 +165,7 @@ export class HubWrappers {
forceDownload: args.deleteFirst,
};

assert.isTrue(HubMock.isValid || openArgs.syncMode === SyncMode.PullOnly, "use HubMock to acquire briefcases");
assert.isTrue(this.hubMock.isValid || openArgs.syncMode === SyncMode.PullOnly, "use HubMock to acquire briefcases");
while (true) {
try {
return (await RpcBriefcaseUtility.open(openArgs)) as BriefcaseDb;
Expand Down Expand Up @@ -254,6 +255,8 @@ export class HubWrappers {

export class IModelTestUtils {

protected static get knownTestLocations(): { outputDir: string, assetsDir: string } { return KnownTestLocations; }

/** Generate a name for an iModel that's unique using the baseName provided and appending a new GUID. */
public static generateUniqueName(baseName: string) {
return `${baseName} - ${Guid.createValue()}`;
Expand All @@ -267,10 +270,10 @@ export class IModelTestUtils {
* @param fileName Name of output fille
*/
public static prepareOutputFile(subDirName: string, fileName: string): LocalFileName {
if (!IModelJsFs.existsSync(KnownTestLocations.outputDir))
IModelJsFs.mkdirSync(KnownTestLocations.outputDir);
if (!IModelJsFs.existsSync(this.knownTestLocations.outputDir))
IModelJsFs.mkdirSync(this.knownTestLocations.outputDir);

const outputDir = path.join(KnownTestLocations.outputDir, subDirName);
const outputDir = path.join(this.knownTestLocations.outputDir, subDirName);
if (!IModelJsFs.existsSync(outputDir))
IModelJsFs.mkdirSync(outputDir);

Expand All @@ -283,7 +286,7 @@ export class IModelTestUtils {

/** Resolve an asset file path from the asset name by looking in the known assets directory */
public static resolveAssetFile(assetName: string): LocalFileName {
const assetFile = path.join(KnownTestLocations.assetsDir, assetName);
const assetFile = path.join(this.knownTestLocations.assetsDir, assetName);
assert.isTrue(IModelJsFs.existsSync(assetFile));
return assetFile;
}
Expand Down
9 changes: 7 additions & 2 deletions core/backend/src/test/LocalHub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
import { LockConflict, LockMap, LockProps, LockState } from "../BackendHubAccess";
import { BriefcaseManager } from "../BriefcaseManager";
import { BriefcaseLocalValue, IModelDb, SnapshotDb } from "../IModelDb";
import { IModelHost } from "../IModelHost";
import { IModelJsFs } from "../IModelJsFs";
import { SQLiteDb } from "../SQLiteDb";

Expand Down Expand Up @@ -105,7 +104,13 @@ export class LocalHub {
db.executeSQL("CREATE INDEX SharedLockIdx ON sharedLocks(briefcaseId)");
db.saveChanges();

const version0 = arg.version0 ?? join(IModelHost.cacheDir, "version0.bim");
const version0Root = `${rootDir}_version0`;

if (!arg.version0) {
IModelJsFs.recursiveMkDirSync(version0Root);
}

const version0 = arg.version0 ?? join(version0Root, "version0.bim");

if (!arg.version0) { // if they didn't supply a version0 file, create a blank one.
IModelJsFs.removeSync(version0);
Expand Down
3 changes: 3 additions & 0 deletions core/backend/src/test/hubaccess/BriefcaseManager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ describe("BriefcaseManager", async () => {
const managerAccessToken = "manager mock token";
const accessToken = "access token";

// contested version0 files can cause errors that cause tests to not call shutdown, so always do it here
afterEach(() => HubMock.shutdown());

it("Open iModels with various names causing potential issues on Windows/Unix", async () => {
HubMock.startup("bad names");
let iModelName = "iModel Name With Spaces";
Expand Down
16 changes: 11 additions & 5 deletions core/transformer/src/IModelTransformer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1206,8 +1206,11 @@ export class IModelTransformer extends IModelExportHandler {
const transformer = new this(...constructorArgs);
const db = new SQLiteDb();
db.openDb(statePath, OpenMode.Readonly);
transformer.loadStateFromDb(db);
db.closeDb();
try {
transformer.loadStateFromDb(db);
} finally {
db.closeDb();
}
return transformer as InstanceType<SubClass>;
}

Expand Down Expand Up @@ -1285,9 +1288,12 @@ export class IModelTransformer extends IModelExportHandler {
if (IModelJsFs.existsSync(nativeStatePath))
IModelJsFs.unlinkSync(nativeStatePath);
db.createDb(nativeStatePath);
this.saveStateToDb(db);
db.saveChanges();
db.closeDb();
try {
this.saveStateToDb(db);
db.saveChanges();
} finally {
db.closeDb();
}
}

/** Export changes from the source iModel and import the transformed entities into the target iModel.
Expand Down
10 changes: 10 additions & 0 deletions core/transformer/src/test/HubMock.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Bentley Systems, Incorporated. All rights reserved.
* See LICENSE.md in the project root for license terms and full copyright notice.
*--------------------------------------------------------------------------------------------*/
import { HubMock as BackendHubMock } from "@itwin/core-backend/lib/cjs/test/HubMock";
import { KnownTestLocations } from "./KnownTestLocations";

export class HubMock extends BackendHubMock {
protected static override get knownTestLocations() { return KnownTestLocations; }
}
Loading

0 comments on commit 9f8cea8

Please sign in to comment.