-
-
Notifications
You must be signed in to change notification settings - Fork 423
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: assign deeply cloned processArguments for starting a WDA session #2245
Conversation
so that it does not push language and locale arguments twice
unfortunately the test failed but it does not likely relate to this PR. |
Do you mean even when you do not set these language/locale, these were added? For example the expected value was |
oo, i see.
so keep growing the arguments |
lib/driver.js
Outdated
@@ -892,7 +892,7 @@ class XCUITestDriver extends BaseDriver { | |||
try { | |||
this.cachedWdaStatus = | |||
this.cachedWdaStatus || (await this.proxyCommand('/status', 'GET')); | |||
await this.startWdaSession(this.opts.bundleId, this.opts.processArguments); | |||
await this.startWdaSession(this.opts.bundleId, _.cloneDeep(this.opts.processArguments)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think startWdaSession
method side should take care of that instead of the caller. Something like:
--- a/lib/driver.js
+++ b/lib/driver.js
@@ -1343,7 +1343,7 @@ class XCUITestDriver extends BaseDriver {
}
async startWdaSession(bundleId, processArguments) {
- const args = processArguments ? processArguments.args || [] : [];
+ const args = processArguments ? _.cloneDeep(processArguments.args) || [] : [];
if (!_.isArray(args)) {
throw new Error(
`processArguments.args capability is expected to be an array. ` +
diff --git a/test/unit/language-specs.js b/test/unit/language-specs.js
index 9014b4d5..7a0e98a8 100644
--- a/test/unit/language-specs.js
+++ b/test/unit/language-specs.js
@@ -121,6 +121,7 @@ describe('language and locale', function () {
proxySpy.calledOnce.should.be.true;
proxySpy.firstCall.args[0].should.eql('/session');
proxySpy.firstCall.args[1].should.eql('POST');
+ desiredCapabilities.processArguments.should.equal(processArguments);
/** @type {any} */ (proxySpy.firstCall.args[2]).should.eql(expectedWDACapabilities);
});
});
then, test case can take care of it as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you! Updated based on your comment
This reverts commit f34b109.
so that it does not affect processArguments at caller side
## [5.9.1](v5.9.0...v5.9.1) (2023-11-21) ### Bug Fixes * assign deeply cloned processArguments for starting a WDA session ([#2245](#2245)) ([2e6f273](2e6f273))
🎉 This PR is included in version 5.9.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
I observed the following appium server log which appium-xcuitest-driver pushes language and locale arguments when retrying
/session
api call to WDA.