Tests: add sidebar tests -p3 #3211

mike10ca · 2024-02-06T14:58:06Z

What it solves

Resolves #3187

How this PR fixes it

Added new tests for sidebar
Added data-testids to support tests

New tests:

Verify Fiat currency changes when edited in the assets tab
Verify "wallet" tag counter if the safe has tx ready for execution
Verify "Wallet" tag counter only shows for owners
Verify New Transaction button enabled for users with Spending limits allowed
Verify tag counting queue tx show for owners and non-owners

How to test it

Run Cypress tests and observe outcome.

github-actions · 2024-02-06T14:58:19Z

Branch preview

✅ Deploy successful!

https://sidebar_tests_p3--walletweb.review-wallet-web.5afe.dev/home?safe=eth:0xA77DE01e157f9f57C7c4A326eeE9C4874D0598b6

github-actions · 2024-02-06T14:59:20Z

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type	Occurrences	Fixable
Errors	0	0
Warnings	0	0
Ignored	0	N/A

Result: ✅ success
Annotations: 0 total

^{Report generated by eslint-plus-action}

github-actions · 2024-02-06T15:01:37Z

📦 Next.js Bundle Analysis for safe-wallet-web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

⚠️ Global Bundle Size Increased

Page	Size (compressed)
`global`	`1.02 MB` (🟡 +21 B)

Details

The global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster.

Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis

If you want further insight into what is behind the changes, give @next/bundle-analyzer a try!

One Page Changed Size

The following page changed size from the code in this PR compared to its base branch:

Page	Size (compressed)	First Load
`/balances`	`27.68 KB` (🟡 +16 B)	1.05 MB

Details

Only the gzipped size is provided here based on an expert tip.

First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If next/link is used, subsequent page loads would only need to download that page's bundle (the number in the "Size" column), since the global bundle has already been downloaded.

Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis

Next to the size is how much the size has increased or decreased compared with the base branch of this PR. If this percentage has increased by 20% or more, there will be a red status indicator applied, indicating that special attention should be given to this.

github-actions · 2024-02-06T15:05:08Z

Coverage report

❌ An unexpected error occurred. For more details, check console

Error: The process '/usr/local/bin/yarn' failed with exit code 1

St.^❔	Category	Percentage	Covered / Total
🟡	Statements	78.85% (+0.01% 🔼)	11271/14294
🔴	Branches	56.59% (-0.02% 🔻)	2504/4425
🟡	Functions	63.2% (+0.02% 🔼)	1795/2840
🟢	Lines	80.2% (+0.02% 🔼)	10165/12674

Show files with reduced coverage 🔻

St.^❔	File	Statements	Branches	Functions	Lines
🟢	`...` / index.ts	91.67%	60% (-10% 🔻)	100%	91.67%

Test suite run failed

Failed tests: 2/1384. Failed suites: 1/186.

  ● useRedefine for SafeTransaction › should return the redefine issues

    expect(received).toBeUndefined()

    Received: [Error: Unavailable]

    Ignored nodes: comments, script, style
    <html>
      <head />
      <body>
        <div />
      </body>
    </html>

      172 |
      173 |     await waitFor(() => {
    > 174 |       expect(result.current[1]).toBeUndefined()
          |                                 ^
      175 |       expect(result.current[0]).toBeDefined()
      176 |       const response = result.current[0]
      177 |       expect(response?.severity).toEqual(SecuritySeverity.LOW)

      at toBeUndefined (src/components/tx/security/redefine/useRedefine.test.ts:174:33)
      at runWithExpensiveErrorDiagnosticsDisabled (node_modules/@testing-library/dom/dist/config.js:47:12)
      at checkCallback (node_modules/@testing-library/dom/dist/wait-for.js:127:77)
      at node_modules/@testing-library/dom/dist/wait-for.js:70:9

  ● useRedefine for SafeTransaction › should poll again on error code 1000

    expect(received).toBeDefined()

    Received: undefined

    Ignored nodes: comments, script, style
    <html>
      <head />
      <body>
        <div />
      </body>
    </html>

      289 |
      290 |     await waitFor(() => {
    > 291 |       expect(result.current[0]).toBeDefined()
          |                                 ^
      292 |       const response = result.current[0]
      293 |       expect(response?.severity).toEqual(SecuritySeverity.LOW)
      294 |       expect(response?.payload?.issues).toHaveLength(1)

      at toBeDefined (src/components/tx/security/redefine/useRedefine.test.ts:291:33)
      at runWithExpensiveErrorDiagnosticsDisabled (node_modules/@testing-library/dom/dist/config.js:47:12)
      at checkCallback (node_modules/@testing-library/dom/dist/wait-for.js:127:77)
      at node_modules/@testing-library/dom/dist/wait-for.js:70:9

Report generated by 🧪jest coverage report action from d049dd4

francovenica

Only the test descriptions are a request to change.
The comments about the data-testid's are a suggestion

francovenica · 2024-02-07T02:27:41Z

cypress/e2e/regression/sidebar_2.cy.js

+    sideBar.checkCurrencyInHeader(constants.currencies.cad)
+  })
+
+  it('Verify "wallet" tag counter if the safe has tx ready for execution', () => {


This test description is not correct. That counter counts how many tx in queue the current connected owner can sign, not necesarely they can be executed.
Change it to something like "Verify "wallet" tag counter of tx that need signature from current owner"

This is original test title taken from our hub of tests and scheduled for automation, the implementation is according to the title. If there are changes needed in updating test title, we can do this later, outside of the scope of this task.

francovenica · 2024-02-07T02:30:00Z

cypress/e2e/regression/sidebar_2.cy.js

+    sideBar.verifyMissingSignature(staticSafe200)
+  })
+
+  it('Verify "Wallet" tag counter only shows for owners', () => {


Same with this description, is not correct. The gray tag with the "check" icon shows how many tx are in queue for everyone (owners, non-owners, disconnected users)

This is original test title taken from our hub of tests and scheduled for automation, the implementation is according to the title. If there are changes needed in updating test title, we can do this later, outside of the scope of this task.

francovenica · 2024-02-07T02:32:59Z

src/components/balances/CurrencySelect/index.tsx

@@ -43,7 +44,7 @@ const CurrencySelect = (): ReactElement => {
        onClose={() => handleTrack('Close')}
      >
        {fiatCurrencies.map((item) => (
-          <MenuItem key={item} value={item} sx={{ overflow: 'hidden' }}>
+          <MenuItem data-testid="currency-item" key={item} value={item} sx={{ overflow: 'hidden' }}>


Not a big fan of having a data test id that should be unique in several elements. Is there a way to have it being "currency-item-1", "currency-item-2", ... ?
Same with other elements like "missing-signature-info" and "queued-tx-info"

Not a blocker if not possible/too cumbersome to do

I suggest to keep these data-testeds as is, this is a common practise to assign data-testid for the same element types which can be accessed through Cypress in more precise way.

tests: add sidebar tests -p3

d049dd4

mike10ca marked this pull request as ready for review February 6, 2024 15:16

mike10ca requested review from katspaugh and francovenica February 6, 2024 15:27

francovenica requested changes Feb 7, 2024

View reviewed changes

francovenica approved these changes Feb 7, 2024

View reviewed changes

katspaugh merged commit 9887d96 into dev Feb 7, 2024
19 of 21 checks passed

katspaugh deleted the sidebar-tests-p3 branch February 7, 2024 14:09

github-actions bot locked and limited conversation to collaborators Feb 7, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tests: add sidebar tests -p3 #3211

Tests: add sidebar tests -p3 #3211

mike10ca commented Feb 6, 2024

github-actions bot commented Feb 6, 2024 •

edited

Loading

github-actions bot commented Feb 6, 2024

github-actions bot commented Feb 6, 2024

github-actions bot commented Feb 6, 2024 •

edited

Loading

francovenica left a comment •

edited

Loading

francovenica Feb 7, 2024

mike10ca Feb 7, 2024

francovenica Feb 7, 2024

mike10ca Feb 7, 2024

francovenica Feb 7, 2024

mike10ca Feb 7, 2024

Tests: add sidebar tests -p3 #3211

Tests: add sidebar tests -p3 #3211

Conversation

mike10ca commented Feb 6, 2024

What it solves

How this PR fixes it

How to test it

github-actions bot commented Feb 6, 2024 • edited Loading

Branch preview

github-actions bot commented Feb 6, 2024

ESLint Summary View Full Report

github-actions bot commented Feb 6, 2024

📦 Next.js Bundle Analysis for safe-wallet-web

⚠️ Global Bundle Size Increased

One Page Changed Size

github-actions bot commented Feb 6, 2024 • edited Loading

Coverage report

Test suite run failed

francovenica left a comment • edited Loading

Choose a reason for hiding this comment

francovenica Feb 7, 2024

Choose a reason for hiding this comment

mike10ca Feb 7, 2024

Choose a reason for hiding this comment

francovenica Feb 7, 2024

Choose a reason for hiding this comment

mike10ca Feb 7, 2024

Choose a reason for hiding this comment

francovenica Feb 7, 2024

Choose a reason for hiding this comment

mike10ca Feb 7, 2024

Choose a reason for hiding this comment

github-actions bot commented Feb 6, 2024 •

edited

Loading

github-actions bot commented Feb 6, 2024 •

edited

Loading

francovenica left a comment •

edited

Loading