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

refactor: getCookies and deleteCookies #1538

Merged
merged 8 commits into from
Apr 18, 2023
Merged

refactor: getCookies and deleteCookies #1538

merged 8 commits into from
Apr 18, 2023

Conversation

KazuCocoa
Copy link
Member

@KazuCocoa KazuCocoa commented Mar 30, 2023 •
edited
Loading

A follow up pr for #1534

  • Tested on simulator that iOS 12 returned {"cookies":[]} so maybe we can remove {"cookies":[]} at least
  • Wrote a single map in getCookies
  • Use B.all in deleteCookies

Example of deleteCookies

[HTTP] --> DELETE /session/d9a75fa5-867d-4122-a6de-a1a0927102b5/cookie
[HTTP] {}
[debug] [XCUITestDriver@f9af (d9a75fa5)] Calling AppiumDriver.deleteCookies() with args: ["d9a75fa5-867d-4122-a6de-a1a0927102b5"]
[debug] [XCUITestDriver@f9af (d9a75fa5)] Executing command 'deleteCookies'
[debug] [RemoteDebugger] Getting cookies
[debug] [RemoteDebugger] Sending '_rpc_forwardSocketData:' message to app 'PID:3566', page '1', target 'page-7' (id: 72): 'Page.getCookies'
[debug] [RemoteDebugger] Received data response from send (id: 72): '{"cookies":[{"name":"key1","value":"value","domain":"0.0.0.0","path":"/","expires":0,"session":true,"httpOnly":false,"secure":false,"sameSite":"None"},{"name":"key10","value":"value","domain":"0.0....'
[debug] [RemoteDebugger] Sending to Web Inspector took 16ms
[debug] [RemoteDebugger] Deleting cookie 'key1' on 'http://0.0.0.0/'
[debug] [RemoteDebugger] Sending '_rpc_forwardSocketData:' message to app 'PID:3566', page '1', target 'page-7' (id: 74): 'Page.deleteCookie'
[debug] [RemoteDebugger] Deleting cookie 'key10' on 'http://0.0.0.0/'
[debug] [RemoteDebugger] Sending '_rpc_forwardSocketData:' message to app 'PID:3566', page '1', target 'page-7' (id: 76): 'Page.deleteCookie'
[debug] [RemoteDebugger] Deleting cookie 'key11' on 'http://0.0.0.0/'
[debug] [RemoteDebugger] Sending '_rpc_forwardSocketData:' message to app 'PID:3566', page '1', target 'page-7' (id: 78): 'Page.deleteCookie'
[debug] [RemoteDebugger] Deleting cookie 'key2' on 'http://0.0.0.0/'
[debug] [RemoteDebugger] Sending '_rpc_forwardSocketData:' message to app 'PID:3566', page '1', target 'page-7' (id: 80): 'Page.deleteCookie'
[debug] [RemoteDebugger] Deleting cookie 'key3' on 'http://0.0.0.0/'
[debug] [RemoteDebugger] Sending '_rpc_forwardSocketData:' message to app 'PID:3566', page '1', target 'page-7' (id: 82): 'Page.deleteCookie'
[debug] [RemoteDebugger] Deleting cookie 'key4' on 'http://0.0.0.0/'
[debug] [RemoteDebugger] Sending '_rpc_forwardSocketData:' message to app 'PID:3566', page '1', target 'page-7' (id: 84): 'Page.deleteCookie'
[debug] [RemoteDebugger] Deleting cookie 'key5' on 'http://0.0.0.0/'
[debug] [RemoteDebugger] Sending '_rpc_forwardSocketData:' message to app 'PID:3566', page '1', target 'page-7' (id: 86): 'Page.deleteCookie'
[debug] [RemoteDebugger] Deleting cookie 'key6' on 'http://0.0.0.0/'
[debug] [RemoteDebugger] Sending '_rpc_forwardSocketData:' message to app 'PID:3566', page '1', target 'page-7' (id: 88): 'Page.deleteCookie'
[debug] [RemoteDebugger] Deleting cookie 'key7' on 'http://0.0.0.0/'
[debug] [RemoteDebugger] Sending '_rpc_forwardSocketData:' message to app 'PID:3566', page '1', target 'page-7' (id: 90): 'Page.deleteCookie'
[debug] [RemoteDebugger] Deleting cookie 'key8' on 'http://0.0.0.0/'
[debug] [RemoteDebugger] Sending '_rpc_forwardSocketData:' message to app 'PID:3566', page '1', target 'page-7' (id: 92): 'Page.deleteCookie'
[debug] [RemoteDebugger] Deleting cookie 'key9' on 'http://0.0.0.0/'
[debug] [RemoteDebugger] Sending '_rpc_forwardSocketData:' message to app 'PID:3566', page '1', target 'page-7' (id: 94): 'Page.deleteCookie'
(node:3097) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 error listeners added to [Socket]. Use emitter.setMaxListeners() to increase limit
(Use `node --trace-warnings ...` to show where the warning was created)
[debug] [RemoteDebugger] Received data response from send (id: 74): '{}'
[debug] [RemoteDebugger] Received data response from send (id: 76): '{}'
[debug] [RemoteDebugger] Received data response from send (id: 78): '{}'
[debug] [RemoteDebugger] Received data response from send (id: 80): '{}'
[debug] [RemoteDebugger] Received data response from send (id: 82): '{}'
[debug] [RemoteDebugger] Received data response from send (id: 84): '{}'
[debug] [RemoteDebugger] Received data response from send (id: 86): '{}'
[debug] [RemoteDebugger] Received data response from send (id: 88): '{}'
[debug] [RemoteDebugger] Received data response from send (id: 90): '{}'
[debug] [RemoteDebugger] Received data response from send (id: 92): '{}'
[debug] [RemoteDebugger] Sending to Web Inspector took 16ms
[debug] [RemoteDebugger] Sending to Web Inspector took 15ms
[debug] [RemoteDebugger] Sending to Web Inspector took 13ms
[debug] [RemoteDebugger] Sending to Web Inspector took 12ms
[debug] [RemoteDebugger] Sending to Web Inspector took 12ms
[debug] [RemoteDebugger] Sending to Web Inspector took 11ms
[debug] [RemoteDebugger] Sending to Web Inspector took 10ms
[debug] [RemoteDebugger] Sending to Web Inspector took 9ms
[debug] [RemoteDebugger] Sending to Web Inspector took 9ms
[debug] [RemoteDebugger] Sending to Web Inspector took 7ms
[debug] [RemoteDebugger] Received data response from send (id: 94): '{}'
[debug] [RemoteDebugger] Sending to Web Inspector took 7ms
[debug] [XCUITestDriver@f9af (d9a75fa5)] Responding to client with driver.deleteCookies() result: true
[HTTP] <-- DELETE /session/d9a75fa5-867d-4122-a6de-a1a0927102b5/cookie 200 35 ms - 14

@KazuCocoa KazuCocoa mentioned this pull request Mar 30, 2023
Merged
@KazuCocoa KazuCocoa changed the title refactor: add todo refactor: modify cookies methods Mar 31, 2023
Base automatically changed from kazucocoa/cookies to master April 1, 2023 02:59
@KazuCocoa KazuCocoa marked this pull request as ready for review April 3, 2023 04:53
@KazuCocoa KazuCocoa changed the title refactor: modify cookies methods refactor: getCookies and deleteCookies Apr 3, 2023
@KazuCocoa KazuCocoa marked this pull request as draft April 3, 2023 21:42
@KazuCocoa
Copy link
Member Author

I'll update a few to follow https://github.com/appium/appium/blob/a11a6ede3320c0a58aad82fd7fc4c8b8ba75ae95/packages/types/lib/driver.ts#L783 and around

@KazuCocoa KazuCocoa marked this pull request as ready for review April 4, 2023 07:04
@github-actions
Copy link
Contributor

PR Preview Action 2a652922e
🚀 Deployed preview to https://appium.github.io/appium-xcuitest-driver/pr-preview/pr-1538/
on branch docs-site at 2023-04-16 02:11 UTC

mykola-mokhnach
mykola-mokhnach reviewed Apr 17, 2023
View reviewed changes
lib/commands/web.js
this.log.warn(error);
return undefined;
return cookies.map((cookie) => {
if (!_.isEmpty(cookie.value)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

we may also apply .filter

mykola-mokhnach
mykola-mokhnach reviewed Apr 17, 2023
View reviewed changes
lib/commands/web.js Outdated
return cookies.map((cookie) => {
if (!_.isEmpty(cookie.value)) {
try {
cookie.value = decodeURI(cookie.value);
Copy link
Contributor

Choose a reason for hiding this comment

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

extra space here

@KazuCocoa KazuCocoa merged commit 14e70b7 into master Apr 18, 2023
@KazuCocoa KazuCocoa deleted the kazucocoa/refactor-cookies branch April 18, 2023 15:54
github-actions bot pushed a commit that referenced this pull request Apr 18, 2023
@semantic-release-bot
chore(release): 4.23.1 [skip ci]
ce76b9c 
## [4.23.1](v4.23.0...v4.23.1) (2023-04-18)

### Bug Fixes

* **command:** flip args for mobile: getPermission ([de44162](de44162))

### Code Refactoring

* getCookies and deleteCookies ([#1538](#1538)) ([14e70b7](14e70b7))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mykola-mokhnach mykola-mokhnach mykola-mokhnach approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@KazuCocoa @mykola-mokhnach