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 template to include test for releaseable builds #92

Merged
merged 2 commits into from
Sep 12, 2018
Merged

Update template to include test for releaseable builds #92

merged 2 commits into from
Sep 12, 2018

Conversation

srirambv
Copy link
Contributor

No description provided.

@srirambv srirambv self-assigned this Sep 10, 2018
@srirambv srirambv requested review from bbondy and kjozwiak September 10, 2018 21:55

### Crash verification
- [ ] Verify these [crash issues](https://github.com/brave/brave-browser/issues?utf8=%E2%9C%93&q=is%3Aissue+milestone%3A%22Releasable+builds+0.55.x%22+label%3Acrash+is%3Aclosed++label%3AQA%2FNeeded) have not regressed
- [ ] Verify these [webview crash issues](https://github.com/brave/brave-browser/issues?utf8=%E2%9C%93&q=is%3Aissue+milestone%3A%22Releasable+builds+0.55.x%22+label%3Acrash/webview+is%3Aclosed++label%3AQA%2FNeeded) have not regressed
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we're at risk in general of the crashes regressing, so I don't think we need to spend time on these on each release.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, not sure if it's the best idea keep going through all those crashes to make sure they haven't regressed. Getting stats.brave into a better state would be a better idea to keep an eye on crash rates coming in on the different channels.

- [ ] Test that an old extension gets updated to a new one from an old profile on Google server
- [ ] Test that an old extension gets updated to a new one from an old profile on Brave server
- [ ] Verify magnet links loads Torrent viewer page and able to download torrent
Copy link
Member

Choose a reason for hiding this comment

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

You can add .torrent files too, that will land probably today.

- [ ] Test that you can log into Netflix and start a show from a fresh profile
### CWS

- [ ] Verify installing uBO/ABP from CWS shows warning message `NOT A RECOMMENDED BRAVE EXTESION!`but still gets installed
Copy link
Member

Choose a reason for hiding this comment

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

I think you can just pick a single extension. Doesn't have to be uBlock or ABP either.


- [ ] Verify installing uBO/ABP from CWS shows warning message `NOT A RECOMMENDED BRAVE EXTESION!`but still gets installed
- [ ] Verify installing LastPass from CWS doesn't show any warning message
- [ ] Verify installing an extesion that is not vetted by Brave gets blocked
Copy link
Member

Choose a reason for hiding this comment

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

not vetted extensions can still be installed, just gives a warning. We don't have any blocked yet so probably just remove this?

Copy link
Member

Choose a reason for hiding this comment

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

I think we should keep this but change the case to make sure that none vetted extensions correctly warn the user before installation. Risk missing a regression where warnings stop working. Could be really bad if users start complaining that Brave didn't warn them and they installed a bad actor extension.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would suggest to test all three allowed/vetted but allowed/ blocked just so we cover all possible scenarios

### PDF

- [ ] Test that you can print a PDF
- [ ] Test that PDF is loaded over https at https://basicattentiontoken.org/BasicAttentionTokenWhitePaper-4.pdf
Copy link
Member

Choose a reason for hiding this comment

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

nit: HTTPS


#### Adblock

- [ ] Verify the execeptions in `brave/common/shield_exceptions.cc` work
Copy link
Member

Choose a reason for hiding this comment

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

We have automated for this so you can probably just remove this.


## Yet to implement

- Keyboard Shortcuts
Copy link
Member

Choose a reason for hiding this comment

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

What's left for that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Custom shortcuts that would be added for Brave. Tor tab, preferences, reopen last window etc...

Copy link
Member

@bbondy bbondy left a comment

Choose a reason for hiding this comment

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

Some inline comments

- [ ] Verify these [webview crash issues](https://github.com/brave/brave-browser/issues?utf8=%E2%9C%93&q=is%3Aissue+milestone%3A%22Releasable+builds+0.55.x%22+label%3Acrash/webview+is%3Aclosed++label%3AQA%2FNeeded) have not regressed


### Installer

- [ ] Check that installer is close to the size of last release.
Copy link
Member

Choose a reason for hiding this comment

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

@bbondy will we be mostly using stub installers? If that's the case, we'll need to change this as it's not really a valid check as the application we'll be pulled/installed from a remote server. If we're going to support both regular binaries and stub installers, we should add another case similar to the following:

  • Install brave using the stub installer and ensure the installed profile is similar in size to the previous version

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I retained that test for the offline installer for Windows. These setup related tests are blocked on brave/omaha#4

- [ ] Verify these [webview crash issues](https://github.com/brave/brave-browser/issues?utf8=%E2%9C%93&q=is%3Aissue+milestone%3A%22Releasable+builds+0.55.x%22+label%3Acrash/webview+is%3Aclosed++label%3AQA%2FNeeded) have not regressed


### Installer

- [ ] Check that installer is close to the size of last release.
- [ ] Check signature: If OS Run `spctl --assess --verbose /Applications/Brave-Browser.app/` and make sure it returns `accepted`. If Windows right click on the installer exe and go to Properties, go to the Digital Signatures tab and double click on the signature. Make sure it says "The digital signature is OK" in the popup window.
Copy link
Member

@kjozwiak kjozwiak Sep 11, 2018

Choose a reason for hiding this comment

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

I know the current QA team will know that the spctl --assess --verbose /Applications/Brave-Browser.app/ might differ while on different channels but we should explicitly mention which locations need to be checked depending on the channel. Example: spctl --assess --verbose /Applications/Brave-Browser-Dev.app/. I think it will also help contributors once we start trying to get more folks involved in brave-core.

@bbondy @srirambv thoughts?

We should also find out a way if there's a way of checking the signatures on Linux (at least Ubunutu).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. changed it to beta since that is where most of the feature/regression fix testing will happen once we have proper channels for testing.

- [ ] Verify these [webview crash issues](https://github.com/brave/brave-browser/issues?utf8=%E2%9C%93&q=is%3Aissue+milestone%3A%22Releasable+builds+0.55.x%22+label%3Acrash/webview+is%3Aclosed++label%3AQA%2FNeeded) have not regressed


### Installer

- [ ] Check that installer is close to the size of last release.
- [ ] Check signature: If OS Run `spctl --assess --verbose /Applications/Brave-Browser.app/` and make sure it returns `accepted`. If Windows right click on the installer exe and go to Properties, go to the Digital Signatures tab and double click on the signature. Make sure it says "The digital signature is OK" in the popup window.
- [ ] Check Brave and Chromium version in chrome://about
Copy link
Member

Choose a reason for hiding this comment

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

Should be chrome://version/ and not chrome://about

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed it as it was checked again in about page section

- [ ] Check that you can import from Google Chrome (We implemented all ourselves)
- [ ] Check that you can import from Firefox (Cookies is all we implemented ourselves)
- [ ] Check that you can import from Edge (No custom code we implemented ourselves)
- [ ] Test chrome://adblock loads adblock page
Copy link
Member

Choose a reason for hiding this comment

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

nit: easier to read while going through the manual passes if formatted chrome://adblock

- [ ] Check that you can import from Firefox (Cookies is all we implemented ourselves)
- [ ] Check that you can import from Edge (No custom code we implemented ourselves)
- [ ] Test chrome://adblock loads adblock page
- [ ] Test chrome://rewards loads Brave rewards page
Copy link
Member

Choose a reason for hiding this comment

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

nit: easier to read while going through the manual passes if formatted chrome://rewards

- [ ] Check that you can import from Edge (No custom code we implemented ourselves)
- [ ] Test chrome://adblock loads adblock page
- [ ] Test chrome://rewards loads Brave rewards page
- [ ] Test chrome://newtab loads a new tab
Copy link
Member

Choose a reason for hiding this comment

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

nit: easier to read while going through the manual passes if formatted chrome://newtab

- [ ] Test chrome://adblock loads adblock page
- [ ] Test chrome://rewards loads Brave rewards page
- [ ] Test chrome://newtab loads a new tab
- [ ] Test chrome://welcome loads the welcome page
Copy link
Member

Choose a reason for hiding this comment

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

nit: easier to read while going through the manual passes if formatted chrome://welcome

- [ ] Test chrome://rewards loads Brave rewards page
- [ ] Test chrome://newtab loads a new tab
- [ ] Test chrome://welcome loads the welcome page
- [ ] Test chrome://version correctly shows Brave version and Chromium version
Copy link
Member

Choose a reason for hiding this comment

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

nit: easier to read while going through the manual passes if formatted chrome://version


- [ ] Test that you can print a PDF
- [ ] Test that PDF is loaded over https at https://basicattentiontoken.org/BasicAttentionTokenWhitePaper-4.pdf
- [ ] Test that PDF is loaded over http at http://www.pdf995.com/samples/pdf.pdf
Copy link
Member

Choose a reason for hiding this comment

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

same nit as @bbondy above: HTTP (protocols should always be capitalized)

@srirambv
Copy link
Contributor Author

ready for review

- Sync
- Tor private tabs
- Manual Update tests from older version
- Test that updating using `BRAVE_UPDATE_VERSION=0.8.3` env variable works correctly.
Copy link
Member

Choose a reason for hiding this comment

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

Is this valid in this codebase? I don't think so.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No just kept it for reference to verify for offline installer once that is available

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants