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

Changes for fallback key test #17

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Changes for fallback key test #17

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
7f27065
Return room-id from create-room action
MidhunSureshR Dec 15, 2022
51d7e79
Fix types
MidhunSureshR Dec 15, 2022
cf50ec6
Send object instead of string
MidhunSureshR Dec 15, 2022
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: 19 additions & 8 deletions cypress/e2e/trafficlight/actions/room.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,31 @@ limitations under the License.

/// <reference types='cypress' />

export function createRoom(name: string, topic: string): string {
cy.get('.mx_RoomListHeader_plusButton').click();
export async function createRoom(name: string, topic: string): Promise<{roomId: string}> {
cy.get('.mx_RoomListHeader_plusButton').click({ force: true });
cy.get('.mx_ContextualMenu').contains('New room').click();
cy.get('.mx_CreateRoomDialog_name input').type(name);
if (topic) {
cy.get('.mx_CreateRoomDialog_topic input').type(topic);
}
// do this to prevent https://github.com/vector-im/element-web/issues/22590, weirdly
// cy.get('.mx_CreateRoomDialog_name input').click();
// cy.wait(5000);

cy.get('.mx_Dialog_primary').click();
//cy.get('.mx_RoomHeader_nametext').should('contain', data['name']);
return "room_created";
const roomId = await getRoomIdFromName(name);
return { roomId };
}

function getRoomIdFromName(name: string): Promise<string> {
let resolve;
const promise: Promise<string> = new Promise(r => resolve = r);
openRoom(name);
cy.get(".mx_RightPanel_roomSummaryButton").click();
cy.get(".mx_RoomSummaryCard_icon_settings").click();
cy.get(`[data-testid='settings-tab-ROOM_ADVANCED_TAB']`).click();
cy.get(".mx_CopyableText").invoke("text").then(roomId => {
cy.get(".mx_Dialog_cancelButton").click();
cy.get("[data-test-id=base-card-close-button]").click();
resolve(roomId);
MidhunSureshR marked this conversation as resolved.
Show resolved Hide resolved
});
return promise;
}

export function createDm(userId: string): string {
Expand Down
6 changes: 3 additions & 3 deletions cypress/e2e/trafficlight/actions/timeline.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,11 @@ export function verifyLastMessageIsTrusted(): string {
return "verified";
}

export function getTimeline(): JSONValue {
const rsp = [];
export function getTimeline(): Record<string, any> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we mean to change JSON value -> Record here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

JSONValue is a type we define in trafficlight.spec.ts and it's not exported. Record here is basically saying that this function returns some object. Ideally we'd define the type explicitly, for eg:

type TimelineContent = { 
    user: string[]; 
    e2e_issues: string[];
}

const rsp: any = [];
Cypress.$('.mx_EventTile').each(
function(index, obj) {
tile = {};
const tile = {};
tile['user'] = Cypress.$(obj).find('.mx_BaseAvatar_image').attr('title');
const e2eicon = Cypress.$(obj).find('.mx_EventTile_e2eIcon').attr('class');
tile['e2e_issues'] = e2eicon;
Expand Down
16 changes: 9 additions & 7 deletions cypress/e2e/trafficlight/trafficlight.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ function recurse() {

function sendResponse(responseStatus) {
let data;
if (typeof responseStatus == "string") {
if (typeof responseStatus === "string") {
data = { response: responseStatus };
} else {
data = responseStatus;
Expand All @@ -121,23 +121,25 @@ function recurse() {
const data: JSONValue = resp.body.data;
const action: string = resp.body.action;
cy.log('running action', action, JSON.stringify(data));
let result;
try {
result = runAction(action, data);
cy.resolveFromPromise(runAction(action, data)).then(result => {
if (result) {
sendResponse(result);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens to the flow if we don't send a result? We need to handle that as an error case (ActionRecord can be of type undefined) otherwise I think we'll end up in a loop doing a poll then trying to perform the action again

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should move the whole set of methods to a promise interface and stop returning undefined; instead making the promise error. IDK.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only certain actions like idle and exit return undefined. All other actions will return something (most likely a string).
If these other actions don't return something that means an error was thrown while they were being executed. We have hooked to the cypress events so that these are caught and reported to the server. See

trafficlight-adapter-element-web/cypress/e2e/trafficlight/trafficlight.spec.ts

Lines 56 to 87 in 51d7e79

Cypress.on('uncaught:exception', (e, runnable) => {
console.log("uncaught exception", e.message);
const errorUrl = `${Cypress.env('TRAFFICLIGHT_URL') }/client/${ Cypress.env('TRAFFICLIGHT_UUID') }/error`;
const errorPath = e.stack?.split("\n")[0];
const body = JSON.stringify({
error: {
type: e.name,
details: e.message,
path: errorPath,
},
});
fetch(errorUrl, { method: "POST", body });
return false;
});
Cypress.on('fail', (e) => {
const errorUrl = `${Cypress.env('TRAFFICLIGHT_URL') }/client/${ Cypress.env('TRAFFICLIGHT_UUID') }/error`;
const errorPath = e.stack?.split("\n").slice(1).join("\n");
const body = JSON.stringify({
error: {
type: e.name,
details: e.message,
path: errorPath,
},
});
fetch(errorUrl, {
method: "post",
body,
headers: { 'Content-Type': 'application/json' },
});
throw e;
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree that this can probably be written in a better way but I suggest that we just create an issue for now (since this code was not introduced by the changes in this PR).

});
} catch (e) {
// Don't keep running if we encounter an error!
return;
}
if (result) {
sendResponse(result);
}
if (action !== 'exit') {
recurse();
}
});
}

function runAction(action: string, data: JSONValue): string | JSONValue | undefined {
type ActionResult = string | JSONValue | undefined;

function runAction(action: string, data: JSONValue): ActionResult | Promise<ActionResult> {
switch (action) {
// Auth
case 'register':
Expand Down
5 changes: 5 additions & 0 deletions cypress/support/commands.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,9 @@ Cypress.Commands.addAll({
cy.get(".mx_UserMenu_userAvatar").click();
cy.get(".mx_IconizedContextMenu_optionList").contains("All settings").click();
},

// A very hacky way to use promises in Cypress
async resolveFromPromise(p: any) {
return await p;
},
});
10 changes: 6 additions & 4 deletions types.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@

/// <reference types="cypress" />

// Add
declare global {
namespace Cypress {
interface Chainable {
/**
* Custom command to go to All Settings view in element web.
*/
gotoAllSettings(): Chainable<Element>;
gotoAllSettings(): Chainable<Element>;
/**
* A very hacky way to use promises in Cypress
*/
resolveFromPromise(p: any): Chainable<any>;
}
}
}

export { };
export {};