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.

Sign up for GitHub

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

Jump to bottom

Fix issue where local extension is incorrectly inferred as published #3499

Merged
merged 3 commits into from
Jun 16, 2021
Merged

Fix issue where local extension is incorrectly inferred as published #3499

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bf4e77c
Fix issue where local extension is incorrectly inferred as published …
huangjeff5 Jun 16, 2021
0c839a9
Add tests
huangjeff5 Jun 16, 2021
b4ef22e
Merge branch 'master' into jh-fix-update-flow
huangjeff5 Jun 16, 2021
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
9 changes: 2 additions & 7 deletions src/extensions/updateHelper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,16 +52,11 @@ export async function getExistingSourceOrigin(
const registryEntry = await resolveSource.resolveRegistryEntry(extensionName);
if (resolveSource.isOfficialSource(registryEntry, existingSource)) {
existingSourceOrigin = SourceOrigin.OFFICIAL_EXTENSION;
} else {
existingSourceOrigin = SourceOrigin.PUBLISHED_EXTENSION;
}
} catch {
// If registry entry does not exist, assume existing source was from local directory or URL.
if (urlRegex.test(existingSource)) {
existingSourceOrigin = SourceOrigin.URL;
} else {
existingSourceOrigin = SourceOrigin.LOCAL;
}
} catch {
existingSourceOrigin = SourceOrigin.LOCAL;
}
return existingSourceOrigin;
}
Expand Down
92 changes: 89 additions & 3 deletions src/test/extensions/updateHelper.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ const INSTANCE = {
state: "ACTIVE",
config: {
name:
"projects/invader-zim/instances/image-resizer/configurations/95355951-397f-4821-a5c2-9c9788b2cc63",
"projects/invader-zim/instances/instance-of-official-ext/configurations/95355951-397f-4821-a5c2-9c9788b2cc63",
createTime: "2019-05-19T00:20:10.416947Z",
sourceId: "fake-official-source",
sourceName: "projects/firebasemods/sources/fake-official-source",
Expand All @@ -120,22 +120,40 @@ const INSTANCE = {
};

const REGISTRY_INSTANCE = {
name: "projects/invader-zim/instances/fake-official-instance",
name: "projects/invader-zim/instances/instance-of-registry-ext",
createTime: "2019-05-19T00:20:10.416947Z",
updateTime: "2019-05-19T00:20:10.416947Z",
state: "ACTIVE",
config: {
name:
"projects/invader-zim/instances/image-resizer/configurations/95355951-397f-4821-a5c2-9c9788b2cc63",
"projects/invader-zim/instances/instance-of-registry-ext/configurations/95355951-397f-4821-a5c2-9c9788b2cc63",
createTime: "2019-05-19T00:20:10.416947Z",
sourceId: "fake-registry-source",
sourceName: "projects/firebasemods/sources/fake-registry-source",
extensionRef: "test-publisher/test",
source: {
name: "projects/firebasemods/sources/fake-registry-source",
},
},
};

const LOCAL_INSTANCE = {
name: "projects/invader-zim/instances/instance-of-local-ext",
createTime: "2019-05-19T00:20:10.416947Z",
updateTime: "2019-05-19T00:20:10.416947Z",
state: "ACTIVE",
config: {
name:
"projects/invader-zim/instances/instance-of-local-ext/configurations/95355951-397f-4821-a5c2-9c9788b2cc63",
createTime: "2019-05-19T00:20:10.416947Z",
sourceId: "fake-registry-source",
sourceName: "projects/firebasemods/sources/fake-local-source",
source: {
name: "projects/firebasemods/sources/fake-local-source",
},
},
};

describe("updateHelper", () => {
describe("updateFromLocalSource", () => {
let promptStub: sinon.SinonStub;
Expand Down Expand Up @@ -563,3 +581,71 @@ describe("inferUpdateSource", () => {
expect(result).to.equal("notfirebase/storage-resize-images@latest");
});
});

describe("getExistingSourceOrigin", () => {
let registryEntryStub: sinon.SinonStub;
let isOfficialStub: sinon.SinonStub;
let getInstanceStub: sinon.SinonStub;

afterEach(() => {
registryEntryStub.restore();
isOfficialStub.restore();
getInstanceStub.restore();
});

it("should return official extension as source origin", async () => {
registryEntryStub = sinon.stub(resolveSource, "resolveRegistryEntry");
registryEntryStub.resolves(REGISTRY_ENTRY);
isOfficialStub = sinon.stub(resolveSource, "isOfficialSource");
isOfficialStub.returns(true);
getInstanceStub = sinon.stub(extensionsApi, "getInstance").resolves(INSTANCE);
const result = await updateHelper.getExistingSourceOrigin(
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Repo convention is to split unit tests into 3 chunks separated by newlines- setup, call functions under test, and expects. For example, in this test, you should add a newline between l601 and l602, and one between l607 and l608.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. Thanks for the heads up!

"invader-zim",
"instance-of-official-ext",
"ext-testing",
"projects/firebasemods/sources/fake-official-source"
);
expect(result).to.equal(extensionsHelper.SourceOrigin.OFFICIAL_EXTENSION);
});

it("should return published extension as source origin", async () => {
registryEntryStub = sinon.stub(resolveSource, "resolveRegistryEntry");
registryEntryStub.throwsException("Entry not found");
getInstanceStub = sinon.stub(extensionsApi, "getInstance").resolves(REGISTRY_INSTANCE);
const result = await updateHelper.getExistingSourceOrigin(
"invader-zim",
"instance-of-registry-ext",
"ext-testing",
"projects/firebasemods/sources/fake-registry-source"
);
expect(result).to.equal(extensionsHelper.SourceOrigin.PUBLISHED_EXTENSION);
});

it("should return local extension as source origin", async () => {
registryEntryStub = sinon.stub(resolveSource, "resolveRegistryEntry");
registryEntryStub.throwsException("Entry not found");
getInstanceStub = sinon.stub(extensionsApi, "getInstance").resolves(LOCAL_INSTANCE);
const result = await updateHelper.getExistingSourceOrigin(
"invader-zim",
"instance-of-local-ext",
"ext-testing",
"projects/firebasemods/sources/fake-local-source"
);
expect(result).to.equal(extensionsHelper.SourceOrigin.LOCAL);
});

it("should return local extension as source origin", async () => {
registryEntryStub = sinon.stub(resolveSource, "resolveRegistryEntry");
registryEntryStub.resolves(REGISTRY_ENTRY);
isOfficialStub = sinon.stub(resolveSource, "isOfficialSource");
isOfficialStub.returns(false);
getInstanceStub = sinon.stub(extensionsApi, "getInstance").resolves(LOCAL_INSTANCE);
const result = await updateHelper.getExistingSourceOrigin(
"invader-zim",
"instance-of-local-ext",
"ext-testing",
"projects/firebasemods/sources/fake-local-source"
);
expect(result).to.equal(extensionsHelper.SourceOrigin.LOCAL);
});
});