From 23018302a74d76add937e80c8c7fff206de1b6c9 Mon Sep 17 00:00:00 2001 From: reluc Date: Fri, 11 Jun 2021 14:47:09 +0200 Subject: [PATCH 1/6] Fix plugin unloding process Fix 2837. Now adapters cannot be added after unload is called --- src/plugin/adapter-proxy.ts | 4 ++-- src/plugin/plugin.ts | 7 +++++++ src/test/integration/internal-logs-test.ts | 8 +++++++- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/plugin/adapter-proxy.ts b/src/plugin/adapter-proxy.ts index 1346c683c..9ca150311 100644 --- a/src/plugin/adapter-proxy.ts +++ b/src/plugin/adapter-proxy.ts @@ -104,8 +104,8 @@ export default class AdapterProxy extends Adapter { */ unload(): Promise { if (this.unloadCompletedPromise) { - console.error('AdapterProxy: unload already in progress'); - return Promise.reject(); + console.warn('AdapterProxy: unload already in progress'); + return this.unloadCompletedPromise.getPromise(); } this.unloadCompletedPromise = new Deferred(); this.sendMsg(MessageType.ADAPTER_UNLOAD_REQUEST, { diff --git a/src/plugin/plugin.ts b/src/plugin/plugin.ts index 154874c61..244289751 100644 --- a/src/plugin/plugin.ts +++ b/src/plugin/plugin.ts @@ -108,6 +108,8 @@ export default class Plugin { private stderrReadline?: readline.Interface; + private unloding = false; + constructor( private pluginId: string, private addonManager: AddonManager, @@ -323,6 +325,10 @@ export default class Plugin { switch (msg.messageType) { case MessageType.ADAPTER_ADDED_NOTIFICATION: { const data = msg.data as AdapterAddedNotificationMessageData; + if (this.unloding) { + console.warn('Adapter', data.adapterId, 'added during unloading'); + return; + } const adapter = new AdapterProxy( this.addonManager, data.adapterId, @@ -862,6 +868,7 @@ export default class Plugin { unload(): void { this.restart = false; + this.unloding = true; this.sendMsg(MessageType.PLUGIN_UNLOAD_REQUEST, {}); } diff --git a/src/test/integration/internal-logs-test.ts b/src/test/integration/internal-logs-test.ts index fd48fa16f..4cc46f5f4 100644 --- a/src/test/integration/internal-logs-test.ts +++ b/src/test/integration/internal-logs-test.ts @@ -12,6 +12,12 @@ describe('internal-logs/', () => { beforeEach(async () => { jwt = await createUser(server, TEST_USER); fs.writeFileSync(path.join(UserProfile.logDir, 'test.log'), 'hello, world!'); + + // clean up folder from previous logs + const regex = /^run-app\.log\./; + fs.readdirSync(UserProfile.logDir) + .filter((f) => regex.test(f)) + .map((f) => fs.unlinkSync(path.join(UserProfile.logDir, f))); }); it('GET internal-logs index', async () => { @@ -55,7 +61,7 @@ describe('internal-logs/', () => { }); expect(res.status).toEqual(200); expect(res.type).toBe('application/zip'); - expect(Object.keys(res.body.files).length).toEqual(2); + expect(Object.keys(res.body.files).length).toEqual(1); const file = res.body.file('logs/test.log'); expect(file).toBeTruthy(); const data = await file.async('text'); From 90a0eb77dc68695d5dbb15a1735476be09a58950 Mon Sep 17 00:00:00 2001 From: reluc Date: Thu, 17 Jun 2021 09:54:24 +0200 Subject: [PATCH 2/6] Add log message when unloding a plugin --- src/plugin/plugin.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/plugin/plugin.ts b/src/plugin/plugin.ts index 244289751..2950018a7 100644 --- a/src/plugin/plugin.ts +++ b/src/plugin/plugin.ts @@ -867,6 +867,7 @@ export default class Plugin { } unload(): void { + console.log("Unloading plugin",this.pluginId) this.restart = false; this.unloding = true; this.sendMsg(MessageType.PLUGIN_UNLOAD_REQUEST, {}); From 4f9ad1fc5c779ddd34c769cb7430d28d14e5e16c Mon Sep 17 00:00:00 2001 From: reluc Date: Thu, 17 Jun 2021 17:38:48 +0200 Subject: [PATCH 3/6] Fix code style --- src/plugin/plugin.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugin/plugin.ts b/src/plugin/plugin.ts index 2950018a7..4b9e17311 100644 --- a/src/plugin/plugin.ts +++ b/src/plugin/plugin.ts @@ -867,7 +867,7 @@ export default class Plugin { } unload(): void { - console.log("Unloading plugin",this.pluginId) + console.log('Unloading plugin', this.pluginId); this.restart = false; this.unloding = true; this.sendMsg(MessageType.PLUGIN_UNLOAD_REQUEST, {}); From bd45ef337dcfe447c082015cc8105ff4f2deb04b Mon Sep 17 00:00:00 2001 From: reluc Date: Wed, 23 Jun 2021 18:16:59 +0200 Subject: [PATCH 4/6] Returns a promise when starting a plugin --- src/addon-manager.ts | 2 +- src/plugin/plugin-server.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/addon-manager.ts b/src/addon-manager.ts index 41f1a7ebd..8b210ed63 100644 --- a/src/addon-manager.ts +++ b/src/addon-manager.ts @@ -678,7 +678,7 @@ export class AddonManager extends EventEmitter { // Load the add-on console.log(`Loading add-on: ${manifest.id}`); - this.pluginServer!.loadPlugin(addonPath, manifest.id, manifest.exec); + await this.pluginServer!.loadPlugin(addonPath, manifest.id, manifest.exec); } /** diff --git a/src/plugin/plugin-server.ts b/src/plugin/plugin-server.ts index 1744d8a92..40fabafad 100644 --- a/src/plugin/plugin-server.ts +++ b/src/plugin/plugin-server.ts @@ -134,11 +134,11 @@ export default class PluginServer extends EventEmitter { * * Loads a plugin by launching a separate process. */ - loadPlugin(pluginPath: string, id: string, exec: string): void { + loadPlugin(pluginPath: string, id: string, exec: string): Promise { const plugin = this.registerPlugin(id); plugin.setExec(exec); plugin.setExecPath(pluginPath); - plugin.start(); + return plugin.start(); } /** From 113470b194edc539d73d9518e5b4dba73ec58435 Mon Sep 17 00:00:00 2001 From: reluc Date: Tue, 29 Jun 2021 15:54:09 +0200 Subject: [PATCH 5/6] Use waitFor* functions to sync load-test --- src/test/browser/load-test.ts | 22 +++++++++++++++++----- src/test/browser/things-view/thing-test.ts | 14 ++++++++++++++ 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/src/test/browser/load-test.ts b/src/test/browser/load-test.ts index 5528a695d..de6ff3674 100644 --- a/src/test/browser/load-test.ts +++ b/src/test/browser/load-test.ts @@ -3,6 +3,7 @@ import { waitForExpect } from '../expect-utils'; import { getBrowser } from './browser-common'; import AddonManager from '../../addon-manager'; +const DEFAULT_CLICKABLE_TIMEOUT = 5000; describe('basic browser tests', () => { afterEach(async () => { try { @@ -29,10 +30,11 @@ describe('basic browser tests', () => { await confirmPassword.setValue('rosebud'); const createUserButton = await browser.$('#create-user-button'); + await createUserButton.waitForClickable({ timeout: DEFAULT_CLICKABLE_TIMEOUT }); await createUserButton.click(); const menuButton = await browser.$('#menu-button'); - await menuButton.waitForExist({ timeout: 5000 }); + await menuButton.waitForClickable({ timeout: DEFAULT_CLICKABLE_TIMEOUT }); await waitForExpect(async () => { const newUrl = await browser.getUrl(); @@ -53,8 +55,8 @@ describe('basic browser tests', () => { } await menuButton.click(); - const settingsMenuItem = await browser.$('#settings-menu-item'); + await settingsMenuItem.waitForClickable({ timeout: DEFAULT_CLICKABLE_TIMEOUT }); await settingsMenuItem.click(); // wait fadeout menu-scrim @@ -76,15 +78,17 @@ describe('basic browser tests', () => { ); const addonSettingsLink = await browser.$('#addon-settings-link'); + await addonSettingsLink.waitForClickable({ timeout: DEFAULT_CLICKABLE_TIMEOUT }); await addonSettingsLink.click(); const discoverAddonsButton = await browser.$('#discover-addons-button'); + await discoverAddonsButton.waitForClickable({ timeout: DEFAULT_CLICKABLE_TIMEOUT }); await discoverAddonsButton.click(); const addonInstallVirtualThingsAdapter = await browser.$( '#addon-install-virtual-things-adapter' ); - await addonInstallVirtualThingsAdapter.waitForExist({ timeout: 5000 }); + await addonInstallVirtualThingsAdapter.waitForClickable({ timeout: 10000 }); await addonInstallVirtualThingsAdapter.click(); // virtual-things-adapter is ~10MB, so it might take some time to install @@ -92,11 +96,13 @@ describe('basic browser tests', () => { await addonDiscoverySettingsAdded.waitForExist({ timeout: 30000 }); const settingsBackButton = await browser.$('#settings-back-button'); + await settingsBackButton.waitForClickable({ timeout: DEFAULT_CLICKABLE_TIMEOUT }); await settingsBackButton.click(); await settingsBackButton.click(); await menuButton.click(); const thingsMenuItem = await browser.$('#things-menu-item'); + await thingsMenuItem.waitForClickable({ timeout: DEFAULT_CLICKABLE_TIMEOUT }); await thingsMenuItem.click(); // wait fadeout menu-scrim @@ -118,7 +124,7 @@ describe('basic browser tests', () => { ); const addButton = await browser.$('#add-button'); - await addButton.waitForDisplayed({ timeout: 5000 }); + await addButton.waitForClickable({ timeout: DEFAULT_CLICKABLE_TIMEOUT }); await addButton.click(); const newThingVirtualThings2SaveButton = await browser.$( @@ -127,10 +133,13 @@ describe('basic browser tests', () => { const newThingVirtualThings9SaveButton = await browser.$( '#new-thing-virtual-things-9 > .new-thing-save-button' ); + await newThingVirtualThings2SaveButton.waitForClickable({ timeout: DEFAULT_CLICKABLE_TIMEOUT }); + await newThingVirtualThings9SaveButton.waitForClickable({ timeout: DEFAULT_CLICKABLE_TIMEOUT }); await newThingVirtualThings2SaveButton.click(); await newThingVirtualThings9SaveButton.click(); const addThingBackButton = await browser.$('#add-thing-back-button'); + await addThingBackButton.waitForClickable({ timeout: DEFAULT_CLICKABLE_TIMEOUT }); await addThingBackButton.click(); let things: ElementArray | null = null; @@ -138,15 +147,17 @@ describe('basic browser tests', () => { things = await browser.$$('.thing'); expect(things!.length).toBe(2); }); - + await things![0].waitForDisplayed({ timeout: DEFAULT_CLICKABLE_TIMEOUT }); await things![0].click(); let link = await things![0].$('.thing-details-link'); + await link.waitForClickable({ timeout: 10000 }); await link.click(); let detailUrl = await browser.getUrl(); expect(detailUrl.endsWith('/things/virtual-things-2')).toBeTruthy(); const backButton = await browser.$('#back-button'); + await backButton.waitForClickable({ timeout: DEFAULT_CLICKABLE_TIMEOUT }); await backButton.click(); const webthingCustomCapability = await browser.$('webthing-custom-capability'); @@ -155,6 +166,7 @@ describe('basic browser tests', () => { things = await browser.$$('.thing'); expect(things!.length).toBe(2); link = await things![1].$('.thing-details-link'); + await link.waitForClickable({ timeout: DEFAULT_CLICKABLE_TIMEOUT }); await link.click(); detailUrl = await browser.getUrl(); expect(detailUrl.endsWith('/things/virtual-things-9')).toBeTruthy(); diff --git a/src/test/browser/things-view/thing-test.ts b/src/test/browser/things-view/thing-test.ts index 0a936f467..cdbbf5a21 100644 --- a/src/test/browser/things-view/thing-test.ts +++ b/src/test/browser/things-view/thing-test.ts @@ -31,6 +31,7 @@ describe('Thing', () => { await addThing(desc); const thingsPage = new ThingsPage(browser); + await thingsPage.wait(); await thingsPage.open(); await thingsPage.waitForThings(); @@ -130,6 +131,7 @@ describe('Thing', () => { await addThing(desc); const thingsPage = new ThingsPage(browser); + await thingsPage.wait(); await thingsPage.open(); await thingsPage.waitForThings(); @@ -189,6 +191,8 @@ describe('Thing', () => { await addThing(desc); const thingsPage = new ThingsPage(browser); + await thingsPage.wait(); + await thingsPage.wait(); await thingsPage.open(); await thingsPage.waitForThings(); @@ -262,6 +266,7 @@ describe('Thing', () => { await addThing(desc); const thingsPage = new ThingsPage(browser); + await thingsPage.wait(); await thingsPage.open(); await thingsPage.waitForOffThings(); @@ -302,6 +307,7 @@ describe('Thing', () => { await addThing(desc); const thingsPage = new ThingsPage(browser); + await thingsPage.wait(); await thingsPage.open(); await thingsPage.waitForOffThings(); @@ -349,6 +355,7 @@ describe('Thing', () => { await addThing(desc); const thingsPage = new ThingsPage(browser); + await thingsPage.wait(); await thingsPage.open(); await thingsPage.waitForOffThings(); @@ -441,6 +448,7 @@ describe('Thing', () => { await addThing(desc); const thingsPage = new ThingsPage(browser); + await thingsPage.wait(); await thingsPage.open(); await thingsPage.waitForOffThings(); @@ -522,6 +530,7 @@ describe('Thing', () => { await addThing(desc); const thingsPage = new ThingsPage(browser); + await thingsPage.wait(); await thingsPage.open(); await thingsPage.waitForOffThings(); @@ -622,6 +631,7 @@ describe('Thing', () => { await addThing(desc); const thingsPage = new ThingsPage(browser); + await thingsPage.wait(); await thingsPage.open(); await thingsPage.waitForOffThings(); @@ -739,6 +749,7 @@ describe('Thing', () => { await addThing(desc); const thingsPage = new ThingsPage(browser); + await thingsPage.wait(); await thingsPage.open(); await thingsPage.waitForOffThings(); @@ -870,6 +881,7 @@ describe('Thing', () => { await addThing(desc); const thingsPage = new ThingsPage(browser); + await thingsPage.wait(); await thingsPage.open(); await thingsPage.waitForOffThings(); @@ -913,6 +925,7 @@ describe('Thing', () => { await addThing(desc); const thingsPage = new ThingsPage(browser); + await thingsPage.wait(); await thingsPage.open(); await thingsPage.waitForThings(); @@ -952,6 +965,7 @@ describe('Thing', () => { await addThing(desc); const thingsPage = new ThingsPage(browser); + await thingsPage.wait(); await thingsPage.open(); await thingsPage.waitForThings(); From 6d379156d2fe42bd24049907844d81614abbb9ed Mon Sep 17 00:00:00 2001 From: reluc Date: Thu, 1 Jul 2021 12:37:46 +0200 Subject: [PATCH 6/6] Wait for emlement to be clickable in openSettings --- src/test/browser/page-object/settings-page.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/browser/page-object/settings-page.ts b/src/test/browser/page-object/settings-page.ts index 3ead21cb1..d21987539 100644 --- a/src/test/browser/page-object/settings-page.ts +++ b/src/test/browser/page-object/settings-page.ts @@ -11,6 +11,7 @@ class SettingSection extends Section { const el = this.rootElement!; const href = await el.getAttribute('href'); const id = await el.getAttribute('id'); + await el.waitForClickable(); await el.click(); switch (id) {