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

Added crash,fingerprint and fixed geo location test #47

Merged
merged 3 commits into from Jan 31, 2018

Conversation

@srirambv
Copy link
Contributor

srirambv commented Jan 29, 2018

Fixes #44 #45 #46

@srirambv srirambv self-assigned this Jan 29, 2018
@srirambv srirambv requested review from kjozwiak and LaurenWags Jan 29, 2018
@@ -69,6 +69,7 @@ _Each start should take less than 7 seconds_
- [ ] opened tabs can be reloaded
- [ ] bookmarks on the bookmark toolbar can be opened
- [ ] bookmarks in the bookmark folder toolbar can be opened
- [ ] ensure upgrade sets fingerprint to `Block all fingerprint` when global fingerprint is enabled

This comment has been minimized.

Copy link
@LaurenWags

LaurenWags Jan 29, 2018

Contributor

Do we only need to check this on update to 0.20.x?

This comment has been minimized.

Copy link
@kjozwiak

kjozwiak Jan 30, 2018

Member

Can you specifically mention 0.19.x -> 0.20.x. After a few 0.20.x hotfixes, we can eventually remove this case as most people would have already migrated from 0.19.x -> 0.20.x.

This comment has been minimized.

Copy link
@srirambv

srirambv Jan 30, 2018

Author Contributor

Sure will add upgrade from 01.9.x to 0.20.x

@@ -56,6 +57,7 @@
- [ ] Verify you are able to sync two devices using the secret code
- [ ] Visit a site on device 1 and change shield setting, ensure that the saved site preference is synced to device 2
- [ ] Enable Browsing history sync on device 1, ensure the history is shown on device 2
- [ ] Clear browsing history on device 1, ensure the history is sync back on device 1 from device 2

This comment has been minimized.

Copy link
@LaurenWags

LaurenWags Jan 29, 2018

Contributor

Did you mean to include this?

This comment has been minimized.

Copy link
@srirambv

srirambv Jan 30, 2018

Author Contributor

Yes added it because of brave/sync#182. Not really sure if that is expected or not but since that's been the behaviour added it.

- [ ] Test that 3rd party storage results are blank at https://jsfiddle.net/7ke9r14a/9/ when 3rd party cookies are blocked and not blank when 3rd party cookies are unblocked.
- [ ] Test that audio fingerprint is blocked at https://audiofingerprint.openwpm.com/ when fingerprinting protection is on.
- [ ] Test that browser is not detected on https://extensions.inrialpes.fr/brave/
### Fingerprint Tests

This comment has been minimized.

Copy link
@LaurenWags

LaurenWags Jan 29, 2018

Contributor

Might want to change this to ## Fingerprint Tests to match the other sections.

This comment has been minimized.

Copy link
@srirambv

srirambv Jan 30, 2018

Author Contributor

Kept it similar to the ledger/media test cases as sub section.

- [ ] Test that 3rd party storage results are blank at https://jsfiddle.net/7ke9r14a/9/ when 3rd party cookies are blocked and not blank when 3rd party cookies are unblocked.
- [ ] Test that audio fingerprint is blocked at https://audiofingerprint.openwpm.com/ when fingerprinting protection is on.
- [ ] Test that browser is not detected on https://extensions.inrialpes.fr/brave/
### Fingerprint Tests

This comment has been minimized.

Copy link
@LaurenWags

LaurenWags Jan 29, 2018

Contributor

Same as other, probably want ## Fingerprint Tests for consistency

@@ -114,7 +115,15 @@ _Each start should take less than 7 seconds_

## Geolocation

- [ ] Check that https://developer.mozilla.org/en-US/docs/Web/API/Geolocation/Using_geolocation works
- [ ] Check that https://developer.mozilla.org/en-US/docs/Web/API/Geolocation/Using_geolocation is blocked due to cross origin iframes

This comment has been minimized.

Copy link
@kjozwiak

kjozwiak Jan 30, 2018

Member

nitpick: use cross-origin


## Crash Reporting

- [ ] Check `chrome://crash` causes the new tab to crash

This comment has been minimized.

Copy link
@kjozwiak

kjozwiak Jan 30, 2018

Member

nitpick: "Check that loading chrome://crash causes the new tab to crash"

## Crash Reporting

- [ ] Check `chrome://crash` causes the new tab to crash
- [ ] Check `chrome://crashes` lists all the crash with a crash id

This comment has been minimized.

Copy link
@kjozwiak

kjozwiak Jan 30, 2018

Member

nitpick: "Check that chrome://crashes lists all the crashes and includes both Crash Report ID & Local Crash ID"

- [ ] Check that https://developer.mozilla.org/en-US/docs/Web/API/Geolocation/Using_geolocation works
- [ ] Check that https://developer.mozilla.org/en-US/docs/Web/API/Geolocation/Using_geolocation is blocked due to cross origin iframes
- [ ] Check that https://browserleaks.com/geo works and shows correct location
- [ ] Check that https://html5demos.com/geo/ works but need not be accurate location

This comment has been minimized.

Copy link
@kjozwiak

kjozwiak Jan 30, 2018

Member

nitpick: "Check that https://html5demos.com/geo/ works but doesn't require an accurate location"

Seems like there's a space between that and the URL.

### Fingerprint Tests
- [ ] Visit https://jsfiddle.net/bkf50r8v/13/, ensure 3 blocked items are listed in shields. Result window should show `got canvas fingerprint 0` and `got webgl fingerprint 00`
- [ ] Test that audio fingerprint is blocked at https://audiofingerprint.openwpm.com/ only when `Block all fingerprinting protection` is on.
- [ ] Test that browser is not detected on https://extensions.inrialpes.fr/brave/

This comment has been minimized.

Copy link
@kjozwiak

kjozwiak Jan 30, 2018

Member

nitpick: probably should add "Brave", example: "Test that the Brave browser isn't detected on https://extensions.inrialpes.fr/brave/"

@@ -123,7 +125,15 @@

## Geolocation

- [ ] Check that https://developer.mozilla.org/en-US/docs/Web/API/Geolocation/Using_geolocation works
- [ ] Check that https://developer.mozilla.org/en-US/docs/Web/API/Geolocation/Using_geolocation is blocked due to cross origin iframes

This comment has been minimized.

Copy link
@kjozwiak

kjozwiak Jan 30, 2018

Member

nitpick: use cross-origin

- [ ] Check that https://developer.mozilla.org/en-US/docs/Web/API/Geolocation/Using_geolocation works
- [ ] Check that https://developer.mozilla.org/en-US/docs/Web/API/Geolocation/Using_geolocation is blocked due to cross origin iframes
- [ ] Check that https://browserleaks.com/geo works and shows correct location
- [ ] Check that https://html5demos.com/geo/ works but need not be accurate location

This comment has been minimized.

Copy link
@kjozwiak

kjozwiak Jan 30, 2018

Member

nitpick: "Check that https://html5demos.com/geo/ works but doesn't require an accurate location"

Seems like there's a space between that and the URL.


## Crash Reporting

- [ ] Check `chrome://crash` causes the new tab to crash

This comment has been minimized.

Copy link
@kjozwiak

kjozwiak Jan 30, 2018

Member

nitpick: "Check that loading chrome://crash causes the new tab to crash"

## Crash Reporting

- [ ] Check `chrome://crash` causes the new tab to crash
- [ ] Check `chrome://crashes` lists all the crash with a crash id

This comment has been minimized.

Copy link
@kjozwiak

kjozwiak Jan 30, 2018

Member

nitpick: "Check that chrome://crashes lists all the crashes and includes both Crash Report ID & Local Crash ID"

### Fingerprint Tests
- [ ] Visit https://jsfiddle.net/bkf50r8v/13/, ensure 3 blocked items are listed in shields. Result window should show `got canvas fingerprint 0` and `got webgl fingerprint 00`
- [ ] Test that audio fingerprint is blocked at https://audiofingerprint.openwpm.com/ only when `Block all fingerprinting protection` is on.
- [ ] Test that browser is not detected on https://extensions.inrialpes.fr/brave/

This comment has been minimized.

Copy link
@kjozwiak

kjozwiak Jan 30, 2018

Member

nitpick: probably should add "Brave", example: "Test that the Brave browser isn't detected on https://extensions.inrialpes.fr/brave/"

@srirambv
Copy link
Contributor Author

srirambv commented Jan 30, 2018

Ready for re-review

@@ -69,6 +69,7 @@ _Each start should take less than 7 seconds_
- [ ] opened tabs can be reloaded
- [ ] bookmarks on the bookmark toolbar can be opened
- [ ] bookmarks in the bookmark folder toolbar can be opened
- [ ] ensure upgrading from `0.19.x` to `0.20.x` sets fingerprint to `Block all fingerprint` when global fingerprint is enabled

This comment has been minimized.

Copy link
@kjozwiak

kjozwiak Jan 31, 2018

Member

nitpick: extra space between to and Block all fingerprint

@@ -15,6 +15,7 @@
- [ ] opened tabs can be reloaded
- [ ] bookmarks on the bookmark toolbar can be opened
- [ ] bookmarks in the bookmark folder toolbar can be opened
- [ ] ensure upgrade sets fingerprint to `Block all fingerprint` when global fingerprint is enabled

This comment has been minimized.

Copy link
@kjozwiak

kjozwiak Jan 31, 2018

Member

nitpick: should be using the same one as wikitemplate-cr-upgrade.md, example:

"ensure upgrading from 0.19.x to 0.20.x sets fingerprint to Block all fingerprint when global fingerprint is enabled"

@kjozwiak kjozwiak merged commit a6fd276 into brave:master Jan 31, 2018
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

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