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 test run generator script #298

Merged
merged 11 commits into from
Jun 17, 2021
Merged

Update test run generator script #298

merged 11 commits into from
Jun 17, 2021

Conversation

srirambv
Copy link
Contributor

@srirambv srirambv commented May 31, 2021

Fixes #183

Changes include:

  • Added individual template files so as to reduce manual edit after creating the test runs
  • Added option for creating Tor test runs
  • Added Android x86 template
  • Update script to handle new Desktop/Android flow
  • Updated desktop test run to include macOS (arm64) & macOS(Intel)

Copy link
Contributor

@stephendonner stephendonner left a comment

Choose a reason for hiding this comment

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

LGTM; thanks for doing this, @srirambv 👍

- [ ] Navigate to `/Users/<user>/Library/Application Support/BraveSoftware/Brave-Browser-<channel>/cldoidikboihgcjfkhdeidbpclkineef/<version>`
- [ ] Run `codesign -vvvv tor-<version-tor>-darwin-brave-<version-brave>` to confirm codesign is valid
- [ ] For MacOS Catalina (10.15+) - Run `spctl -a -vv -t install tor-<version-tor>-darwin-brave-<version-brave>` to verify that the binary is notarized.

Copy link
Member

Choose a reason for hiding this comment

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

Not sure if copy/paste error, but Catalina isn't listed, should be between Big Sur and Mojave

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was copy paste from one of the test runs.

brave_testrun_generator.py Show resolved Hide resolved
brave_testrun_generator.py Show resolved Hide resolved
@kjozwiak
Copy link
Member

kjozwiak commented Jun 9, 2021

@srirambv there's also an issue when attempting to generate passes for the current 1.25.x - Release #5 HF. Example:

After selecting 2. Desktop Hotfix via the menu, it will start generating the manual passes as per the following:

Create test runs for:

1. Desktop Release
2. Desktop Hotfix
3. Android Release
4. Android Hotfix
5. iOS Release
6. Tor Release

Choose the platform for which you want to generate the test run: 2

Generating test runs for 1.25.x - Release #5

However, it runs into the following issue and never ends up creating the GH issues:

Traceback (most recent call last):
  File "brave_testrun_generator.py", line 902, in <module>
    laptop_hf_testruns(sorted(bc_milestone.keys())[0])
  File "brave_testrun_generator.py", line 284, in laptop_hf_testruns
    bc_repo.create_issue(title=macTitle,
  File "/home/kjozwiak/.local/lib/python3.8/site-packages/github/Repository.py", line 1218, in create_issue
    headers, data = self._requester.requestJsonAndCheck(
  File "/home/kjozwiak/.local/lib/python3.8/site-packages/github/Requester.py", line 353, in requestJsonAndCheck
    return self.__check(
  File "/home/kjozwiak/.local/lib/python3.8/site-packages/github/Requester.py", line 378, in __check
    raise self.__createException(status, responseHeaders, output)
github.GithubException.GithubException: 422 {"message": "Validation Failed", "errors": [{"value": "s", "resource": "Issue", "field": "assignees", "code": "invalid"}], "documentation_url": "https://docs.github.com/rest/reference/issues#create-an-issue"}

It looks like github.GithubException.GithubException is returning a 422 which seems like we're passing incorrect/or malformed values into the GH API hence the issues not being created.

Definitely not an authentication issues as I managed to create the manual passes for iOS without any issues.

@kjozwiak kjozwiak self-requested a review June 9, 2021 20:24
@kjozwiak
Copy link
Member

kjozwiak commented Jun 9, 2021

I personally think we can remove Per release speciality tests part of the manual passes as @LaurenWags does a good job at managing the current stats re: remaining verification for desktop. It could still be helpful for iOS but here's why I think we should just completely remove it:

  • I find myself using the list generated via a GH search query more often then I ever did via Per release speciality tests
  • the Per release speciality tests list is usually outdated by the time we start running through passes unless we create the passes a few hours before we start running through them which would have an updated list
  • just another extra step re: checking off items that were completed. Adding QA-Pass to the issue should be good enough
  • if an issue changes state re: moving from QA/Yes -> QA/No or QA/Test-All-Platforms is removed, the Per release speciality tests becomes stale as well

I think most of the team knows how to use GH search queries to check and see what's left in each milestone for a specific platform. @LaurenWags @GeetaSarvadnya @StephenBass @btlechowski thoughts?

@kjozwiak
Copy link
Member

kjozwiak commented Jun 9, 2021

@srirambv still reviewing, it will probably take a bit as there's a lot of changes and we'll probably want to make sure these are actually being created on GH to flush out possible issues as mentioned via #298 (comment).

@LaurenWags
Copy link
Member

++ on removing Per release speciality tests, I also use the queries in GH more than the list on the test run.

@srirambv
Copy link
Contributor Author

#298 (comment)

This is fixed. Was an issue with assignee's list. Also generated the test runs via the script https://github.com/brave/brave-browser/milestones/1.25.x%20-%20Release%20%235

#298 (comment)

This is also fixed. Removed the Per release checklist. Agree on list being reduced or outdated by the time manual pass is done. Since manual pass is usually right before the release where all release checklist is more or less completed so removed it completely for all platforms.

@kjozwiak
Copy link
Member

kjozwiak commented Jun 10, 2021

Thoughts not pre-assigning passes? Most of the time everyone knows the pass they're going to run through so they can just assign it to themselves when they start them. There's cases where someone else grabs them instead of the pre-assigned person. For example, I usually run through the Intel pass in the AM before @stephendonner gets online due to the timezone difference when we're doing a HF.

Another example would be iOS passes. Sometimes someone else might run through the ones that have already been assigned to @srirambv if @srirambv is running through verification or is busy doing something else.

I think the general rule should be: If you're starting a manual pass, you assign it to yourself so the QA team knows that someone claimed the work/pass and is running through it. It also might avoid confusion as well re: someone thinking that because it's already assigned, it means they can't start it even though the person that is assigned the pass might not even have plans to start it any time soon. @LaurenWags @GeetaSarvadnya @stephendonner @btlechowski thoughts?

@srirambv
Copy link
Contributor Author

I don't think the issue assignment's a big deal. Its pretty trivial. Everybody knows what they usually run so its easier to assign when they start. And since we always communicate internally when someone else is being asked to run they can just self assign then and remove the original assignee. I am ok to remove it if everyone agrees but feel its unnecessary. We anyways do assign now even when we create the issues manually

@btlechowski
Copy link

++ on removing Per release speciality tests

@LaurenWags
Copy link
Member

my test runs usually come pre-assigned, but I figured that's just because I'm currently the only person with M1 and Android x86 😄 I'm fine either way re: assignment and happy to go with whatever is easiest for all.

@kjozwiak
Copy link
Member

kjozwiak commented Jun 10, 2021

I don't think the issue assignment's a big deal. Its pretty trivial. Everybody knows what they usually run so its easier to assign when they start. And since we always communicate internally when someone else is being asked to run they can just self assign then and remove the original assignee. I am ok to remove it if everyone agrees but feel its unnecessary. We anyways do assign now even when we create the issues manually

I only assign x86 and M1 as @LaurenWags is the only one that has the ability to run through those. @stephendonner will be getting an M1 device shortly so that just leaves x86 with @LaurenWags.

Lets just remove the pre-assigns as it's more intuitive rather than re-assigning. We can leave @LaurenWags as the assignee for x86 and everything else can be set when someone start running through the passes.

@kjozwiak
Copy link
Member

Lets remove Custom tabs, Settings and Bottom bar & Brave Shields from the x86 pass and just leave Installer & Upgrade. Talked to @bbondy and he's fine with it. Just need to do a quick spot check to make sure you can install/upgrade without any issues.

@srirambv
Copy link
Contributor Author

Ok removed default assignee for all test runs. people can self assign when they run. Also updated x86 test template. Should be good enough now

@kjozwiak
Copy link
Member

@srirambv small nit: mind adding the feature/tor label to the manual passes for Tor? BTW, generator looks good when creating Tor passes. Ran through it and created https://github.com/brave/brave-browser/milestone/210 👍

I'll create manual passes for 1.26 today for both Android & Desktop and that looks good, we should be good to merge this. Awesome work!

@srirambv
Copy link
Contributor Author

Added Tor labels. Should be ready for merge

@kjozwiak
Copy link
Member

@srirambv need to change the label for the arm64 pass to OS/macOS-arm64 from OS/macOS.

@srirambv
Copy link
Contributor Author

Done

@LaurenWags
Copy link
Member

@srirambv could you update the Brave Rewards/Ads section of wikitemplate-ios.md to be the reduced set we've been using lately? Example:

## Brave Rewards/Ads

- [ ] Verify wallet is auto-created after enabling rewards
- [ ] Verify when you click on the BR panel while on a site, the panel displays if the site is verified or not
- [ ] Verify ads are only shown when the app is being used
- [ ] Verify ad notification are shown based on ads per hour setting
- [ ] Verify clicking on an ad notification shows the landing page
- [ ] Verify `view`,`clicked` and `landed` and `dismiss` states are logged based on the action

reference: https://bravesoftware.slack.com/archives/GAA4017R7/p1623759655029000

@kjozwiak
Copy link
Member

@srirambv can you update the Tor passes for macOS? Basically can copy what's in brave/brave-browser#16411. Changes:

@LaurenWags
Copy link
Member

LaurenWags commented Jun 15, 2021

@srirambv while @kjozwiak was helping me get set up we talked about needing to change the Installer section for macOS (intel) only, however macOS/Windows/Linux all use the same wikitemplate.md and wikitemplate-minorCRbumpDesktop.md files. We might need to break macOS (intel) out into its own files like arm64 is.

This is the "installer" section macOS (intel) needs:

### Installer

- [ ]  Check signature: If OS Run `"spctl --assess --verbose /Applications/Brave Browser.app"` and make sure it returns `accepted`.
- [ ] Ensured that the following executables work as expected
   - [ ] `Brave-Browser-universal.dmg`
      - [ ] Check executable size, should be `~470mb`
   - [ ] `Brave-Browser-universal.pkg`
       - [ ] Check executable size, should be `~470mb`
   - [ ] `Brave-Browser-x64.dmg`
       - [ ] Check executable size, should be `~270mb`
   - [ ] `Brave-Browser-x64.pkg`
       - [ ] Check executable size, should be `~270mb`
- [ ] Visited `brave://version` and ensure the following:
   - [ ] `x86_64` is being displayed when installing via `x64` or `universal` binaries on `Intel` macOS

wdyt?

@kjozwiak
Copy link
Member

@srirambv lets rename the Tor title for Win to the following:

Manual test run on Windows x64 & x86

Example: brave/brave-browser#16413

@srirambv srirambv mentioned this pull request Jun 17, 2021
@srirambv
Copy link
Contributor Author

I'd suggest we address the template changes in #312 with a follow up PR. This one is more for the script. I updated the Tor and desktop suggestions. If there is nothing else I'd like to get this merged and do a follow up for other changes.

@srirambv srirambv requested a review from LaurenWags June 17, 2021 03:23
Copy link
Member

@kjozwiak kjozwiak left a comment

Choose a reason for hiding this comment

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

LGTM! Nice work. We'll handle the template changes in another PR as you mentioned above.

@srirambv srirambv merged commit cc1c34b into brave:master Jun 17, 2021
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.

Fix test script for Android
5 participants