Skip to content

Conversation

erwinheitzman
Copy link
Member

No description provided.

@christian-bromann
Copy link
Member

@erwinheitzman thanks for raising the PR.

The idea of this recipe project is to have example recipes that are being tested. You see that all examples in here are somewhat connected to a test that runs in CI to ensure that the WebdriverIO examples actually work. Hence I would suggest to only add code samples that we actually have a way to verify that they are working. Wdyt?

Copy link
Member

@wswebcreation wswebcreation left a comment

Choose a reason for hiding this comment

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

I have some small 🐜 ... points 🤣

//...
user: '<browserstack_username>' || process.env.BROWSERSTACK_USERNAME,
key: '<browserstack_access_key>' || process.env.BROWSERSTACK_ACCESS_KEY,
commonCapabilities: {
Copy link
Member

Choose a reason for hiding this comment

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

Never seen commonCapabilities, is this new?

browserName: 'chrome',
'custom:caps': {
// custom configurations
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
}
}
// ...

For consistency

'custom:caps': {
// custom configurations
}
}]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
}]
}]
// ...

For consistency

Comment on lines +11 to +13
securityToken: "your security token"
}],
})
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
securityToken: "your security token"
}],
})
securityToken: "your security token"
// ...
}],
// ...
})

For consistency

Comment on lines +4 to +7
runner: ['browser', {
// network IP of the machine that runs the WebdriverIO process
host: 'http://172.168.0.2'
}]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
runner: ['browser', {
// network IP of the machine that runs the WebdriverIO process
host: 'http://172.168.0.2'
}]
// ...
runner: ['browser', {
// network IP of the machine that runs the WebdriverIO process
host: 'http://172.168.0.2'
}],
// ...

For consistency

Comment on lines +4 to +5
outputDir: 'trace',
// ...
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
outputDir: 'trace',
// ...
// ...
outputDir: 'trace',

For consistency

Comment on lines +10 to +16
browserName,
'goog:chromeOptions': {
// given your wdio.conf.js is in the root directory and your compiled
// web extension files are located in the `./dist` folder
args: [`--load-extension=${path.join(__dirname, '..', '..', 'dist')}`]
}
}]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
browserName,
'goog:chromeOptions': {
// given your wdio.conf.js is in the root directory and your compiled
// web extension files are located in the `./dist` folder
args: [`--load-extension=${path.join(__dirname, '..', '..', 'dist')}`]
}
}]
browserName,
'goog:chromeOptions': {
// given your wdio.conf.js is in the root directory and your compiled
// web extension files are located in the `./dist` folder
args: [`--load-extension=${path.join(__dirname, '..', '..', 'dist')}`],
// ...
}
}]
// ...

For consistency

Comment on lines +10 to +16
// ...
capabilities: [{
browserName,
'goog:chromeOptions': {
extensions: [chromeExtension]
}
}]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// ...
capabilities: [{
browserName,
'goog:chromeOptions': {
extensions: [chromeExtension]
}
}]
// ...
capabilities: [{
browserName,
'goog:chromeOptions': {
extensions: [chromeExtension]
}
}],
// ...

For consistency

Comment on lines +10 to +16
before: async (capabilities) => {
const browserName = (capabilities as WebdriverIO.Capabilities).browserName
if (browserName === 'firefox') {
const extension = await fs.readFile(extensionPath)
await browser.installAddOn(extension.toString('base64'), true)
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
before: async (capabilities) => {
const browserName = (capabilities as WebdriverIO.Capabilities).browserName
if (browserName === 'firefox') {
const extension = await fs.readFile(extensionPath)
await browser.installAddOn(extension.toString('base64'), true)
}
}
before: async (capabilities) => {
const browserName = (capabilities as WebdriverIO.Capabilities).browserName
if (browserName === 'firefox') {
const extension = await fs.readFile(extensionPath)
await browser.installAddOn(extension.toString('base64'), true)
}
},
// ...

For consistency

// ...
before: () => {
browser.addCommand('openExtensionPopup', openExtensionPopup)
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
}
}
// ...

For consistency

@erwinheitzman
Copy link
Member Author

@christian-bromann i just realised that if I put these scripts in the main WebdriverIO repo and link to them from the same repo it should probably also work. Let me move them there as it's much better. I didn't take into account the intent/scope of the repo too well, thanks

@christian-bromann
Copy link
Member

I think some of these examples could be maybe transformed into executable tests. The test doesn't need to do much other other then run the code involved in a somwhat meaningful way. So maybe we can keep some here that would be valuable but yes, you can also keep them within the core repo when they only have documentation purpose.

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.

3 participants