Skip to content

/ servo

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

Add webdriver commands for (Get|Add)Cookie #10826

Merged
merged 1 commit into from Jun 26, 2016
Merged

Add webdriver commands for (Get|Add)Cookie #10826

merged 1 commit into from Jun 26, 2016

Conversation

@dlrobertson
Copy link
Contributor

dlrobertson commented Apr 24, 2016

Add the webdriver commands for GetCookie and AddCookie. In addition the basic messages for sending cookie data back and forth from the resource thread needed to be created and the handlers for those messages were created as well.

Do we plan to have some sort of test suite for the webdriver at some point? At this point I primarily test my PRs with basic shell scripts with a lot of curl and jq.

This change is Reviewable
@highfive
Copy link

highfive commented Apr 24, 2016

Heads up! This PR modifies the following files:

  • @jgraham: components/webdriver_server/lib.rs
  • @KiChjang: components/script/dom/document.rs, components/script/webdriver_handlers.rs, components/net/resource_thread.rs, components/net_traits/lib.rs, components/net_traits/lib.rs, components/net/cookie_storage.rs, components/script/script_thread.rs
@highfive
Copy link

highfive commented Apr 24, 2016

warning Warning warning

  • These commits modify net and script code, but no tests are modified. Please consider adding a test!
@jdm
Copy link
Member

jdm commented Apr 24, 2016

Getting a test harness for the webdriver code is the subject of an ongoing student project.
@dlrobertson dlrobertson force-pushed the dlrobertson:cookies branch from f45ea6b to b680e73 Apr 24, 2016
@dlrobertson
Copy link
Contributor Author

dlrobertson commented Apr 24, 2016

Ah! Thanks! Thats awesome!
@dlrobertson dlrobertson force-pushed the dlrobertson:cookies branch from b680e73 to d612634 Apr 24, 2016
@dlrobertson
Copy link
Contributor Author

dlrobertson commented Apr 25, 2016 •
edited

I didn't correctly handle max-age and expiry per RFC 6265 correctly. I'll make the update shortly
@dlrobertson dlrobertson force-pushed the dlrobertson:cookies branch 2 times, most recently from 6b6e1cd to 91de1f5 Apr 25, 2016
@dlrobertson
Copy link
Contributor Author

dlrobertson commented Apr 25, 2016 •
edited

Fixed. Some of the changes made to max-age and expiry may not be as visible until mozilla/webdriver-rust#29 lands.
@bors-servo
Copy link
Contributor

bors-servo commented Apr 25, 2016

☔ The latest upstream changes (presumably #10696) made this pull request unmergeable. Please resolve the merge conflicts.
@dlrobertson dlrobertson force-pushed the dlrobertson:cookies branch from 91de1f5 to 0b68918 Apr 25, 2016
@KiChjang
Copy link
Member

KiChjang commented Apr 26, 2016

r? @jgraham
@highfive highfive assigned jgraham and unassigned KiChjang Apr 26, 2016
@dlrobertson dlrobertson force-pushed the dlrobertson:cookies branch 2 times, most recently from 64f7c57 to 58c65d2 Apr 26, 2016
@jdm
Copy link
Member

jdm commented Apr 26, 2016

Note, there's a compile error in net/resource_thread.rs.
@dlrobertson dlrobertson force-pushed the dlrobertson:cookies branch from 58c65d2 to 00358d4 Apr 26, 2016
@dlrobertson
Copy link
Contributor Author

dlrobertson commented Apr 26, 2016

Thanks, sorry about that.
@bors-servo
Copy link
Contributor

bors-servo commented May 10, 2016

☔ The latest upstream changes (presumably #11049) made this pull request unmergeable. Please resolve the merge conflicts.
@dlrobertson dlrobertson force-pushed the dlrobertson:cookies branch 2 times, most recently from de881d0 to 88f90cf May 21, 2016
@dlrobertson
Copy link
Contributor Author

dlrobertson commented Jun 25, 2016

@bors-servo try
@bors-servo
Copy link
Contributor

bors-servo commented Jun 25, 2016

⌛ Trying commit 2467231 with merge 4e74967...
bors-servo added a commit that referenced this pull request Jun 25, 2016
@bors-servo
Auto merge of #10826 - danlrobertson:cookies, r=<try>
4e74967 
Add webdriver commands for (Get|Add)Cookie

Add the webdriver commands for GetCookie and AddCookie. In addition the basic messages for sending cookie data back and forth from the resource thread needed to be created and the handlers for those messages were created as well.

Do we plan to have some sort of test suite for the webdriver at some point? At this point I primarily test my PRs with basic shell scripts with a lot of `curl` and `jq`.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10826)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jun 25, 2016

💔 Test failed - linux-rel
@highfive
Copy link

highfive commented Jun 25, 2016 

  ▶ FAIL [expected PASS] /css-transforms-1_dev/html/transform-table-007.htm
  └   → /css-transforms-1_dev/html/transform-table-007.htm a5c014b20ef1363bea6f24eda28c7efb7c45698a
/css-transforms-1_dev/html/reference/transform-blank-ref.htm fa6407b1acbbfea27e27061e7d1bdeca98e4a728
Testing a5c014b20ef1363bea6f24eda28c7efb7c45698a == fa6407b1acbbfea27e27061e7d1bdeca98e4a728
@cbrewster cbrewster removed the S-fails-tidy label Jun 25, 2016
@cbrewster
Copy link
Member

cbrewster commented Jun 26, 2016

@bors-servo r=asajeffrey
@bors-servo
Copy link
Contributor

bors-servo commented Jun 26, 2016

📌 Commit 2467231 has been approved by asajeffrey
@cbrewster cbrewster closed this Jun 26, 2016
@cbrewster cbrewster reopened this Jun 26, 2016
@cbrewster
Copy link
Member

cbrewster commented Jun 26, 2016

@bors-servo r=asajeffrey
@bors-servo
Copy link
Contributor

bors-servo commented Jun 26, 2016

💡 This pull request was already approved, no need to approve it again.

  • This pull request previously failed. You should add more commits to fix the bug, or use retry to trigger a build again.
@bors-servo
Copy link
Contributor

bors-servo commented Jun 26, 2016

📌 Commit 2467231 has been approved by asajeffrey
@Ms2ger
Copy link
Contributor

Ms2ger commented Jun 26, 2016

@bors-servo retry
@bors-servo
Copy link
Contributor

bors-servo commented Jun 26, 2016

⌛ Testing commit 2467231 with merge 054bb38...
bors-servo added a commit that referenced this pull request Jun 26, 2016
@bors-servo
Auto merge of #10826 - danlrobertson:cookies, r=asajeffrey
Loading status checks…
054bb38 
Add webdriver commands for (Get|Add)Cookie

Add the webdriver commands for GetCookie and AddCookie. In addition the basic messages for sending cookie data back and forth from the resource thread needed to be created and the handlers for those messages were created as well.

Do we plan to have some sort of test suite for the webdriver at some point? At this point I primarily test my PRs with basic shell scripts with a lot of `curl` and `jq`.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10826)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jun 26, 2016

💔 Test failed - arm64
@jdm
Copy link
Member

jdm commented Jun 26, 2016

@bors-servo: retry
@bors-servo
Copy link
Contributor

bors-servo commented Jun 26, 2016

⚡ Previous build results for arm32, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows are reusable. Rebuilding only android, arm64...
@bors-servo
Copy link
Contributor

bors-servo commented Jun 26, 2016

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows
@bors-servo bors-servo merged commit 2467231 into servo:master Jun 26, 2016
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
This was referenced Jun 26, 2016
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@asajeffrey asajeffrey

Labels
S-tests-failed
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

9 participants
@dlrobertson @highfive @jdm @bors-servo @KiChjang @asajeffrey @cbrewster @Ms2ger @jgraham
You can’t perform that action at this time.