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

Fix WebdriverIO configs #3663

Merged
merged 2 commits into from
Sep 24, 2024
Merged

Fix WebdriverIO configs #3663

merged 2 commits into from
Sep 24, 2024

Conversation

briehl
Copy link
Member

@briehl briehl commented Sep 24, 2024

Description of PR purpose/changes

The last batch of JS updates played some havoc with the webdriverio configs.
Mainly, in a previous major version of wdio, they started wrapping the most popular browser drivers into wdio itself, which made the wdio-chromedriver-service obsolete. Which, it hasn't been updated in 2 years and has been marked obsolete on Github, so that's a good thing anyway.

But the configs needed a little patching and updating to tell wdio we're not using that service anymore.

Eventually, we'll need to do the same for the standalone-selenium-service (which was put in to talk to firefox), and do some fiddling with browserstack (which I don't think we're actually using).

Chrome alone gets used in GHA, so that gets priority here.

There's a bunch of other little bits of polish to add to the config situation, too, but that's a later project. This does need to fixed, though to get integration tests passing again, and so I can put in more to handle this auth business.

Copy link

@@ -16,7 +16,7 @@ describe('Narrative tree page with login', () => {
});

it('opens a narrative', async () => {
await browser.url(makeURL(`narrative/${testCases.CASE_1.narrativeId}`));
Copy link
Member Author

Choose a reason for hiding this comment

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

makeURL (which just prepends the BASE_URL) isn't necessary. wdio does that for us. Was fiddling around here when I found that - will update other specs in a later PR.

const env = process.env.ENV || 'ci';
const testData = require('./narrative_basic_data.json');
const testCases = testData.envs[env];

describe('Narrative tree page with login', () => {
beforeEach(async () => {
await browser.setTimeout({ implicit: 30000 });
Copy link
Member Author

Choose a reason for hiding this comment

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

implicit timeouts shouldn't be used, according to docs.

@@ -4,8 +4,6 @@
const testConfig = require('../testConfig');
const fs = require('fs');

const CHROME_BINARY = require('puppeteer').executablePath();
Copy link
Member Author

Choose a reason for hiding this comment

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

wdio fetches whatever version you tell it to, now, and does the pathing itself.

This might also make the puppeteer dependency obsolete, will check in a later PR.

Comment on lines +196 to +197
'headless',
'disable-gpu',
Copy link
Member Author

Choose a reason for hiding this comment

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

Updating args according to docs.

return {
browserName: 'chrome',
browserVersion: '128',
Copy link
Member Author

@briehl briehl Sep 24, 2024

Choose a reason for hiding this comment

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

latest version of Chrome. Not sure yet if we can add a spread of these (or if we need to, I think our policy is to stick to the latest).

That should probably get configured elsewhere rather than as a magic number here, but... later.

},
'wdio:enforceWebDriverClassic': true
Copy link
Member Author

Choose a reason for hiding this comment

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

The latest wdio uses the w3-specced webdriver BiDi protocol. There's something strange with either our configuration, or the mash of versions of things, that prevents that protocol from loading pages from our app. So forcing it back to webDriverClassic fixes that (at least, until they ditch that all together).

@@ -322,7 +317,7 @@ const wdioConfig = {
// and 30 processes will get spawned. The property handles how many capabilities
// from the same test should run tests.
//
maxInstances: 10,
maxInstances: 1,
Copy link
Member Author

Choose a reason for hiding this comment

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

It'd be really nice to have this run parallel, but those runs seem to clobber each other. I suspect shenanigans with cookies getting globally added and removed, and communication with the single narrative application we have running. I think it's worth trying to fix that by ensuring that each test spec only uses a single narrative, but this PR's just about returning to status quo.

@@ -392,7 +387,6 @@ const wdioConfig = {
// Services take over a specific job you don't want to take care of. They enhance
// 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: [[testParams.SERVICE, serviceConfigs[testParams.SERVICE]]],
Copy link
Member Author

Choose a reason for hiding this comment

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

These told wdio to use the chromedriver service, which doesn't exist anymore, and was crashing.

Copy link

codecov bot commented Sep 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 25.89%. Comparing base (59479be) to head (d4530b4).
Report is 4 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #3663      +/-   ##
===========================================
- Coverage    25.93%   25.89%   -0.05%     
===========================================
  Files          461      461              
  Lines        46652    46652              
===========================================
- Hits         12099    12079      -20     
- Misses       34553    34573      +20     

see 3 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9fef1d0...d4530b4. Read the comment docs.

@briehl briehl merged commit 1f28a09 into develop Sep 24, 2024
7 of 9 checks passed
@briehl briehl deleted the sep-2024-updates-js branch September 24, 2024 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants