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

feat(webdriverio): Add webdriverio end to end test framework. #11016

Open
wants to merge 43 commits into
base: master
Choose a base branch
from

Conversation

tudor-phd
Copy link
Contributor

No description provided.

Copy link
Member

@saghul saghul left a comment

Choose a reason for hiding this comment

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

Very good start Tudor! 🚀 I left some comments after an initial pass.

package.json Outdated
"@wdio/allure-reporter": "7.16.14",
"@wdio/cli": "7.16.15",
"@wdio/junit-reporter": "7.16.15",
"@wdio/local-runner": "^7.16.13",
Copy link
Member

Choose a reason for hiding this comment

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

Make sure we used specific versions, no ^

package.json Outdated
@@ -179,6 +189,7 @@
"lint-fix": "eslint --max-warnings 0 --fix .",
"postinstall": "patch-package && jetify",
"validate": "npm ls",
"start": "make dev"
"start": "make dev",
"wdio": "wdio run wdio.conf.js"
Copy link
Member

Choose a reason for hiding this comment

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

Let's call this e2e-tests


## Steps to generate allure reports:
* Check if the <i>allure-results</i> was generated
* In order to generate the <i>allure-reports</i>, run the following command: allure generate allure-results/ && allure open
Copy link
Member

Choose a reason for hiding this comment

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

Add a script in package.json to do this please. Maybe some e2ee-test-reports

@@ -0,0 +1 @@
export const BASE_URL = 'https://localhost:8080';
Copy link
Member

Choose a reason for hiding this comment

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

This needs to be configurable, from an environment variable, with a sane default.

@@ -0,0 +1,37 @@
export default async function openSession(participant) {
Copy link
Member

Choose a reason for hiding this comment

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

Docs!

wdio.conf.js Outdated
// Define all options that are relevant for the WebdriverIO instance here
//
// Level of logging verbosity: trace | debug | info | warn | error | silent
logLevel: 'info',
Copy link
Member

Choose a reason for hiding this comment

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

Don't we want more?

wdio.conf.js Outdated
// with `/`, the base url gets prepended, not including the path portion of your baseUrl.
// If your `url` parameter starts without a scheme or `/` (like `some/path`), the base url
// gets prepended directly.
baseUrl: 'http://localhost',
Copy link
Member

Choose a reason for hiding this comment

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

We need to take this value from an env var.

baseUrl: 'http://localhost',
//
// Default timeout for all waitFor* commands.
waitforTimeout: 10000,
Copy link
Member

Choose a reason for hiding this comment

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

IMHO every waitFor should be explicit, waiting for a long time everywhere sounds not so great.

wdio.conf.js Outdated
// your test setup with almost no effort. Unlike plugins, they don't add new
// commands. Instead, they hook themselves up into the test process.
//services: ['chromedriver'],
services: ['selenium-standalone'],
Copy link
Member

Choose a reason for hiding this comment

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

Is this where we could plug the grid?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. The configuration for the grid will look like this -> example:
// Remote selenium grid.
hostname: 'selenium-grid-link',
port: 4444,
path: '/wd/hub/',
protocol: 'http',

Copy link
Member

Choose a reason for hiding this comment

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

But can you pass that as command-line arguments?

Copy link
Member

Choose a reason for hiding this comment

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

Jenkins needs to set it somehow without changing the source code.

//
// Make sure you have the wdio adapter package for the specific framework installed
// before running any tests.
framework: 'mocha',
Copy link
Member

Choose a reason for hiding this comment

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

I see there are other options. Any reason why you went with this one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason I chose this is because the wdio documentation and most of the existing examples focus on mocha and it is easier for me to follow what I need to do. I did not notice a major difference between the options we have available.

package.json Outdated
"start": "make dev"
"start": "make dev",
"e2e-tests": "wdio run wdio.conf.js",
"e2e-test-reports": "npx wdio run ./wdio.conf.js && allure generate allure-results/ --clean && allure open"
Copy link
Member

Choose a reason for hiding this comment

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

Let's make sure reports are always created, but don't try to open them because this will run on a CI 99% of the time.

@@ -0,0 +1,19 @@
import { remote } from 'webdriverio'
export default async function createFirefoxSession() {
const browser = await remote({
Copy link
Member

Choose a reason for hiding this comment

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

No need to await here, you can just return.

const ParticipantsPane = require("../page-objects/ParticipantsPane");
const Toolbox = require("../page-objects/Toolbox")
export default async function openParticipantsPane() {
const toolbox = await Toolbox.ToolboxView;
Copy link
Member

Choose a reason for hiding this comment

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

This reads super weird because you are awaiting a property. Please call it something like getXXX

// Participants pane object.
get ParticipantsPaneView() {
const participantsPane = $('.participants_pane');
return participantsPane
Copy link
Member

Choose a reason for hiding this comment

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

Unneeded variable


// Participants pane object.
get ParticipantsPaneView() {
const participantsPane = $('.participants_pane');
Copy link
Member

Choose a reason for hiding this comment

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

Please don't use jQuery, we want to kill it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Screenshot 2022-03-16 at 15 41 19

default:
roomName = 'SafariRoomNameTest'
}
await browser.url(`${BASE_URL}/${roomName}?${DEFAULT_CONFIG}`);
Copy link
Member

Choose a reason for hiding this comment

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

Please space things out a bit, this is very hard to read.

default:
roomName = 'SafariRoomNameTest'
}
await browser.url(`${BASE_URL}/${roomName}?${DEFAULT_CONFIG}`);
Copy link
Member

Choose a reason for hiding this comment

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

This should probably be a helper.

});
it('should open jitsi-meet with same room name where second participant wants to join', async () => {

switch (capabilities.browserName) {
Copy link
Member

Choose a reason for hiding this comment

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

The decision of the type of browser to use must come from an env var.

let capabilities;
let Guest1;
it('should open jitsi-meet app and enable lobby by first participant', async () => {
capabilities = await browser.requestedCapabilities;
Copy link
Member

Choose a reason for hiding this comment

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

What is this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed. Just tested something related to browser capabilities.

describe('Activate lobby and admit participant', () => {
let roomName;
let capabilities;
let Guest1;
Copy link
Member

Choose a reason for hiding this comment

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

Please use the same terminology as we do in torture: Participant.

tudordan7 added 24 commits March 16, 2022 16:44
@tudor-phd tudor-phd marked this pull request as ready for review March 16, 2022 15:36
Copy link

This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale label Dec 27, 2023
@saghul saghul added confirmed Confirmed bug, should not go stale and removed stale labels Dec 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed Confirmed bug, should not go stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants