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

Update BAT tests #17

Merged
merged 4 commits into from Oct 31, 2017
Merged

Update BAT tests #17

merged 4 commits into from Oct 31, 2017

Conversation

@srirambv
Copy link
Contributor

srirambv commented Oct 10, 2017

Closes #7

@srirambv srirambv self-assigned this Oct 10, 2017
@srirambv srirambv requested review from kjozwiak and LaurenWags Oct 10, 2017
@LaurenWags
Copy link
Contributor

LaurenWags commented Oct 17, 2017

++ looks good to me, will wait for @kjozwiak to give input.

Copy link
Member

kjozwiak left a comment

Looks good other than a few nitpicks like extra lines... Pulled PR and tested, looks like everything was generated correctly.. Bugs are being included etc..

Also compared wikitemplate-cr-upgrade to the old Wiki and ensured we're not missing any cases etc..


## Update tests

- [ ] Test that updating using `BRAVE_UPDATE_VERSION=0.-3` env variable works correctly.

This comment has been minimized.

Copy link
@kjozwiak

kjozwiak Oct 24, 2017

Member

nitpick: not 100% sure if BRAVE_UPDATE_VERSION will work correctly with a - character... We should revert this back to BRAVE_UPDATE_VERSION=0.8.3.... Unless of course, this was intended.

Example of the previous Update tests:

https://github.com/brave/browser-laptop/wiki/Manual-Tests-Plan-for-Chromium-Updates/4a4da6a85b7a3ce9e4f815a9f7425d8f9eca5f91


args = parser.parse_args()


This comment has been minimized.

Copy link
@kjozwiak

kjozwiak Oct 24, 2017

Member

nitpick: remove extra line

args = parser.parse_args()



This comment has been minimized.

Copy link
@kjozwiak

kjozwiak Oct 24, 2017

Member

nitpick: remove extra line

- [ ] Only import a large set of bookmarks.
- [ ] Combine sync, payments, and a large set of bookmarks.

## Ledger

This comment has been minimized.

Copy link
@kjozwiak

kjozwiak Oct 24, 2017

Member

nitpick: add space below ## Ledger to keep document consistent

- [ ] Check that disabling payments and enabling them again does not lose state.


## Sync

This comment has been minimized.

Copy link
@kjozwiak

kjozwiak Oct 24, 2017

Member

nitpick: add space below ## Sync to keep document consistent

- [ ] Test that clicking a bookmark in the toolbar loads the bookmark.
- [ ] Test that clicking a bookmark in a bookmark toolbar folder loads the bookmark.

## Context menus

This comment has been minimized.

Copy link
@kjozwiak

kjozwiak Oct 24, 2017

Member

nitpick: add space below ## Context menus to keep document consistent

- [ ] Test that PDF is loaded at http://www.orimi.com/pdf-test.pdf
- [ ] Test that https://mixed-script.badssl.com/ shows up as grey not red (no mixed content scripts are run).

## Flash tests

This comment has been minimized.

Copy link
@kjozwiak

kjozwiak Oct 24, 2017

Member

nitpick: add space below ## Flash tests to keep document consistent

- [ ] Test that flash placeholder appears on http://www.homestarrunner.com
- [ ] Test with flash enabled in preferences, auto play option is shown when visiting http://www.homestarrunner.com

## Autofill tests

This comment has been minimized.

Copy link
@kjozwiak

kjozwiak Oct 24, 2017

Member

nitpick: add space below ## Autofill tests to keep document consistent

- [ ] Change min visit and min time in advance setting and verify if the publisher list gets updated based on new setting
- [ ] Visit nytimes.com for a few seconds and make sure it shows up in the Payments table.
- [ ] Check that disabling payments and enabling them again does not lose state.

This comment has been minimized.

Copy link
@kjozwiak

kjozwiak Oct 24, 2017

Member

nitpick: remove extra space

@srirambv
Copy link
Contributor Author

srirambv commented Oct 24, 2017

@kjozwiak updated as per feedback

@kjozwiak
Copy link
Member

kjozwiak commented Oct 30, 2017

@srirambv looks like the PR has some conflicts, mind rebasing? Let me know if you need any help with it.

srirambv and others added 2 commits Oct 24, 2017
@kjozwiak kjozwiak merged commit db37151 into brave:master Oct 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.