docs: branch review #5339
docs: branch review #5339
Conversation
jaffrepaul
force-pushed
the
branch-review
branch
from
bf6ece9 to
b1e0159
Compare
jaffrepaul
force-pushed
the
branch-review
branch
from
c11690e to
2efa90c
Compare
✅ Deploy Preview for benevolent-cat-040f48 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
docs/guides/cloud/branch-review.mdx
Outdated
| :::info | ||
|
|
||
| The arrows next to the numbers in each of the tabs indicate: | ||
|
|
||
| - <svg | ||
| width="16" | ||
| height="16" | ||
| viewBox="0 0 16 16" | ||
| fill="none" | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| > | ||
| <path | ||
| d="M12 10V4M12 4L6 4M12 4L4 12" | ||
| stroke="#C62B49" | ||
| stroke-width="2" | ||
| stroke-linecap="round" | ||
| stroke-linejoin="round" | ||
| /> | ||
| </svg> total number increased in your branch | ||
| - <svg | ||
| width="16" | ||
| height="16" | ||
| viewBox="0 0 16 16" | ||
| fill="none" | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| > | ||
| <path | ||
| d="M12 6V12M12 12L6 12M12 12L4 4" | ||
| stroke="#C62B49" | ||
| stroke-width="2" | ||
| stroke-linecap="round" | ||
| stroke-linejoin="round" | ||
| /> | ||
| </svg> total number decreased in your branch | ||
| - <svg | ||
| width="16" | ||
| height="16" | ||
| viewBox="0 0 16 16" | ||
| fill="none" | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| > | ||
| <path | ||
| d="M9 12L13 8M13 8L9 4M13 8L3 8" | ||
| stroke="#C62B49" | ||
| stroke-width="2" | ||
| stroke-linecap="round" | ||
| stroke-linejoin="round" | ||
| /> | ||
| </svg> there are new items, but the total count remained the same - for example, <em> | ||
| 1 resolved and 1 added | ||
| </em> | ||
|
|
||
| ::: |
There was a problem hiding this comment.
the sidebar indicators are quite different now
docs/guides/cloud/branch-review.mdx
Outdated
|
|
||
| ### Review Test Comparison | ||
|
|
||
| Navigating into the test detail view reveals a side-by-side comparison of the test in each branch which can help elevate the source of flake within tests throughout different attempts. Review the test definition panel for diff snapshots to help quickly determine changes in test code. |
There was a problem hiding this comment.
"elevate the source of flake" - this is part of it, but perhaps more importantly it shows you the test results on both branches as well as artifacts so you can compare before vs after your PR
docs/guides/cloud/branch-review.mdx
Outdated
| ### Review Header | ||
|
|
||
| At the left side of the header is the Git commit message, PR selector, PR status and base and target branch indicators. If there are multiple pull requests open for the same branch, you can select the pull request you want to review from the dropdown to the right of the commit message. | ||
|
|
||
| The right side of the header includes the Cypress Cloud test run number and test summary. Clicking the test run number will link you directly to the [test overview](/guides/cloud/recorded-runs#Overview-tab). | ||
|
|
||
| <DocsImage | ||
| src="/img/guides/cloud/branch-review/branch-review-header.jpg" | ||
| alt="Branch Review Header" | ||
| /> |
There was a problem hiding this comment.
the header has been updated and now has tooltips for both runs, which we should document
docs/guides/cloud/branch-review.mdx
Outdated
|
|
||
| ### Review Test Status | ||
|
|
||
| When on the review screen, you will see tabs Failures, Flaky, Pending, Added, and Modified. Each tab will show you the tests that fall into that category. You can click on a test to view the test details. |
There was a problem hiding this comment.
we should also document the subsections of "new"/"existing"/"previous/resolved" – these are probably one of the most confusing parts of the feature, what falls under each
docs/guides/cloud/branch-review.mdx
Outdated
| sidebar_position: 35 | ||
| --- | ||
|
|
||
| Cypress Branch Review allows you to quickly identify the impact a pull request might have on your test suite in a single view. Compare which tests are failing, flaky, pending, added, or modified between the source and base branches. |
There was a problem hiding this comment.
perhaps we use some of the content from the PR/FAQ (currently WIP) here
jaffrepaul
force-pushed
the
branch-review
branch
2 times, most recently
from
c9d95a8 to
9509eca
Compare
docs/guides/cloud/branch-review.mdx
Outdated
| <DocsImage | ||
| src="/img/guides/cloud/branch-review/get-to-branch-review.jpg" | ||
| alt="Get to Branch Review" | ||
| /> |
There was a problem hiding this comment.
Maybe use an image with PR tags on the branches? We may want to link to docs (or briefly mention) about sending PR numbers to runs.
There was a problem hiding this comment.
There was a problem hiding this comment.
I'll look to get this in a state where PR tags are represented.
docs/guides/cloud/branch-review.mdx
Outdated
| <hr /> | ||
|
|
||
| Text references also capture these states on each spec in the right panel. For example: | ||
|
|
||
| - **_new_** = was **not** previously failing, but now **is** failing | ||
| - **_existing_** = **was** previously pending and is now **still** pending | ||
| - **_resolved_** = **was** previously flaky and is now **no longer** flaky | ||
|
|
||
| _note: states may be failed, flaky, or pending_ |
There was a problem hiding this comment.
not sure if there's a more succinct way to present this, but feels pretty hefty here
There was a problem hiding this comment.
Agreed. Having to explain the nuance of a product UI in the Docs is usually a sign things are amiss somewhere. Can try to iterate a bit here.
docs/guides/cloud/branch-review.mdx
Outdated
| fill="#C62B49" | ||
| /> | ||
| </svg> | ||
| total number increased in your branch |
There was a problem hiding this comment.
maybe "added" or "introduced" or similar instead of "increased"? not sure what wording is the most clear
There was a problem hiding this comment.
Yeah we can tighten this up. This is largely the wording provided for the previous UI elements.
jaffrepaul
force-pushed
the
branch-review
branch
6 times, most recently
from
8b879ef to
94b907a
Compare
jaffrepaul
force-pushed
the
branch-review
branch
2 times, most recently
from
52d6892 to
7247035
Compare
docs/guides/cloud/branch-review.mdx
Outdated
| --- | ||
| title: Branch Review | ||
| sidebar_position: 35 | ||
| sidebar_class_name: beta |
There was a problem hiding this comment.
Now that I think about this more, in considering added a beta label here, we should probably either 1. add a beta label for ALL beta features such as Bitbucket and Teams or 2. not have a label. I am leaning towards 2, since we do not want to attract attention to the fact that our other features are beta, as they are already mature.
There was a problem hiding this comment.
Would the label reading "New" be a more useful context as we roll out more of these Cloud intelligence features?
There was a problem hiding this comment.
Yeah, probably. We may want to throw a New on Test Replay too in this case.
docs/guides/cloud/branch-review.mdx
Outdated
| :::caution | ||
|
|
||
|  <Icon name="exclamation-triangle" /> Branch Review is currently in beta. | ||
| See [how to enable](/guides/cloud/branch-review#Public-Beta-Access) this feature below. | ||
|
|
||
| ::: |
There was a problem hiding this comment.
Caution style feels a bit too "warning"y here
docs/guides/cloud/branch-review.mdx
Outdated
|
|
||
| :::info | ||
|
|
||
| Cypress Branch Review is currently only available for [GitHub integrations](/guides/cloud/integrations/source-control/github). We are working on adding support for GitLab and Bitbucket soon. |
There was a problem hiding this comment.
maybe "GitHub integrated projects" or "projects that use our GitHub integration" or similar
docs/guides/cloud/branch-review.mdx
Outdated
|
|
||
| ## Getting Started | ||
|
|
||
| Branch Review works by leveraging the Cypress Cloud Github integration to query the Github API for **open** pull requests on active branches. |
There was a problem hiding this comment.
Important: we retrieve and show ALL PRs, not just open ones
There was a problem hiding this comment.
And not just "active" branches (as "active" is defined on Branches page that is)
docs/guides/cloud/branch-review.mdx
Outdated
|
|
||
| Branch Review is a powerful tool to compare two branches with recorded runs to Cypress Cloud. There are factors that can impact what is available to review between a source and base branch. | ||
|
|
||
| Most importanly, unless a PR is open (not closed or merged), the branch review is not guaranteed to show you the latest run recorded for the branch. When a PR transitions into a closed/merged state, it does not consider any subsequent commits pushed to the branch as part of the PR itself. Be sure that the latest CI job has been triggered or you may see the _closest_ commit and not the one that was _just_ merged. |
There was a problem hiding this comment.
- typo: importanly
- we typically are able to show the latest state for a PR even if it was closed or merged, as long as there were runs on the relevant commits for the PR
- we don't use any "closest" commit logic for Branch Review – if a run is not found for master commit, we omit master data; if a run is not found for feat commit, we use the latest run on feat – we never try to do any location of commits that are "close" to the commits that we find
There was a problem hiding this comment.
I just want to make sure we're all aligned on point number 2 above here. Has this logic changed since last week? From what I gather over slack, there could be some confusion over what might be showing in this scenario. The goal was to get ahead of seeing data that seems out of sync with the user's branches.
cc @tbiethman
There was a problem hiding this comment.
@pstakoun for point number 3, what you describe is what I was getting at. "Closest" probably is misleading. I'll reword that.
There was a problem hiding this comment.
Opted to streamline this a bit as to not list specifics of edge cases. Especially with some logic updates on deck.
docs/guides/cloud/branch-review.mdx
Outdated
|
|
||
| Cypress Cloud allows for [grouping recorded tests](/guides/cloud/smart-orchestration/parallelization#Grouping-test-runs) together under a single run. This means multiple `cypress run` calls can be labeled and associated to a single run in Cypress Cloud. | ||
|
|
||
| It's important to avoid recording random sets of tests between branches. Recording multiple runs per commit without grouping can cause issues, as Branch Review relies on the _latest_ runs. Therefore, recording one run per commit and utilizing grouping to ensure accurate comparisons during branch reviews is recommended to improve effectiveness. |
There was a problem hiding this comment.
Not even just "random", but any disjoint sets. Recording multiple runs on a commit will screw things up 100%. Any scenario where multiple runs happen on a single commit, grouping should just be used to group them into a single run. (CI retries are the only case where obviously they should not be grouped, and we will just pull the latest run which should work for that use case).
There was a problem hiding this comment.
I think the concept is there but getting muddy as-is. I'll try a less is more approach here to see if we feel the scenario is covered.
docs/guides/cloud/branch-review.mdx
Outdated
|
|
||
| Previously, pinpointing changes in your test suite's results required a manual side-by-side comparison between your newly-introduced branch's test runs and your base branch's test runs. This sub-optimal workflow often fails to answer the fundamental questions, _what changed_ and _why?_ You might be left wondering if the same tests are flaky between branches, when new failures were introduced, or if you added sufficient test coverage to your new branch. | ||
|
|
||
| Cypress Branch Review is designed to elevate your pull request (PR) review workflow. It allows you to quickly identify the impact a pull request might have on your test suite in a single view. Compare which tests are failing, flaky, pending, added, or modified between the **<u>source</u>** and **<u>base</u>** branches and prevent the merging of low-quality code. |
There was a problem hiding this comment.
I see we use the phrase source branch throughout; any thoughts on instead making this feature branch or PR branch? The term source isn't immediately clear to me.
There was a problem hiding this comment.
I had the same thought. I think while "source" is certainly technically correct, it's more unversally clear to state "feature" branch. Wanted to run that by @pstakoun
There was a problem hiding this comment.
I'll address the common scenario as the feature -> main branch and replace the other refs.
docs/guides/cloud/branch-review.mdx
Outdated
|
|
||
| ### Review Header | ||
|
|
||
| The header includes the Git commit message, PR selector, PR status, and base and target branch labels with the Cypress Cloud test run ID number (#). |
There was a problem hiding this comment.
We don't use target to refer to the source/feature/PR branch anywhere else in these docs from what I can tell. We should update this to whatever broad term we end up using.
jaffrepaul
force-pushed
the
branch-review
branch
from
15da677 to
3d3dcb2
Compare
docs/guides/cloud/branch-review.mdx
Outdated
|
|
||
| ### Accessing Branch Review | ||
|
|
||
| To access Branch Review, navigate to the **Branches** tab in the left sidebar and select the branch you want to review. In order to see a comparison, the branch you select must have an **open** pull request. |
There was a problem hiding this comment.
all PRs have comparisons if we can find runs for them, not just open ones – for example, if you just merged a PR, you can still go back and see the comparison
docs/guides/cloud/branch-review.mdx
Outdated
|
|
||
| #### Latest Runs | ||
|
|
||
| If the selected branch has a PR associated with it, there is an additional callout on the top of the overview tab of the **Latest runs** page directs to the Branch Review. |
There was a problem hiding this comment.
maybe specify that this is iff PR number is passed via CI
|
|
||
| ### View Options | ||
|
|
||
| "Review" is the default view of the **Branches** tab for projects with linked GitHub repositories via our GitHub integration. |
There was a problem hiding this comment.
note: we will be adding a new Overview tab before we go GA
|
@jaffrepaul how do you feel about an ETA to merge this in? |
Co-authored-by: Peter Stakoun <[email protected]>
jaffrepaul
force-pushed
the
branch-review
branch
from
b5f6c7c to
1d30441
Compare
No description provided.