Skip to content

[C-4228] Add component test framework#8004

Merged
dylanjeffers merged 2 commits into
mainfrom
dj-c-4228-component-tests
Apr 3, 2024
Merged

[C-4228] Add component test framework#8004
dylanjeffers merged 2 commits into
mainfrom
dj-c-4228-component-tests

Conversation

@dylanjeffers

Copy link
Copy Markdown
Contributor

Description

Adds component tests to web using vite and @testing-library/react.
A few changes:

  1. Upgrades vitest to latest
  2. Upgrades @testing-library/react to latest
  3. Fixes bugs from migration
  4. Adds testing-library/jest-dom for extended matchers
  5. Adds custom "vitest-canvas-mock" based on https://github.com/wobsoriano/vitest-canvas-mock which is needed for harmony to work (lottie complains)
  6. Adds custom @audius/test entry point for provider wrapped render
  7. Adds a simple EditTrackForm test to confirm things work

How Has This Been Tested?

npm run test passes

@dylanjeffers dylanjeffers requested a review from sliptype April 3, 2024 00:33
@dylanjeffers dylanjeffers changed the title [C-4228] Add component tests [C-4228] Adds component test framework Apr 3, 2024
@socket-security

socket-security Bot commented Apr 3, 2024

Copy link
Copy Markdown

No dependency changes detected. Learn more about Socket for GitHub ↗︎

👍 No dependency changes detected in pull request

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: worried slightly about name pollution with this - we already have an EditTrackForm in upload

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

that will be moving over here shortly :)

describe('EditTrackForm', () => {
it('renders correctly', () => {
render(
<ThemeProvider theme='day'>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If using the custom render, why add the provider?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes great catch, forgot to remove :)

import { ThemeProvider } from '@audius/harmony'
import { describe, it, expect } from 'vitest'

// import { render, screen } from '@testing-library/react'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

super nit, but probably should put this in a folder somewhere?

Also, super nitpicky but I'm not completely sold on this pattern - I do like that it reduces boilerplate, but seems somewhat "magical" /"clever"? I'm not sure it's immediately obvious what should be in @audius/test (do vitest things live there too? what about playwright? why not?) Maybe it makes sense to have a test folder with various test utilities like the custom render (or even just the testing-specific provider wrapper which can be composed), rather than defining something like @audius/test (which looks like a new package) and re-exporting react testing library?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

down to have further discussion, but this is essentially ripped from https://testing-library.com/docs/react-testing-library/setup, we could have folks manually import, but given tests can be anywhere in codebase, an absolute path may be best, and so to make things "official" i went with @audius/test, can be whatever we think though

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

as far as whether or not to use custom wrapper, i think it's quite beneficial when we start layering things on like redux store, makes it easy to pass initial state to render function instead of having to build everything from scratch

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I agree with having a folder for these test utils, less indirection and things should auto-import just fine

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yeah given there's a few of these files, it'd be nice to home them in a directory together

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yeah totes agree! thoughts on name? tests or vitest?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

test imo

@audius-infra

Copy link
Copy Markdown
Collaborator

Preview this change https://demo.audius.co/dj-c-4228-component-tests

@dylanjeffers dylanjeffers changed the title [C-4228] Adds component test framework [C-4228] Add component test framework Apr 3, 2024

@sliptype sliptype left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Awesome work!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

test imo

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I agree with having a folder for these test utils, less indirection and things should auto-import just fine

const customRender = (
ui: ReactElement,
options?: Omit<RenderOptions, 'wrapper'>
) => render(ui, { wrapper: TestProviders, ...options })

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice

import { describe, it, expect } from 'vitest'

// import { render, screen } from '@testing-library/react'
import { render, screen } from '@audius/test'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

love the use of @audius/test rather than a relative path ❤️

@@ -0,0 +1,24 @@
import { vi } from 'vitest'

@DejayJD DejayJD Apr 3, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ugh I still want to eventually get browser component testing working so we don't have to mock stuff like this 😢

@DejayJD DejayJD left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! Excited to keep the testing momentum going, hoping to start getting some much needed integration suites up and running

I'm still hoping we can eventually swap to a browser-based testing tool (pw) but that definitely shouldn't stop us moving forward with the easiest path forward!
That way we can keep investing in writing tests and if we want to change later in theory it should be relatively easy to port over the tests.

@DejayJD

DejayJD commented Apr 3, 2024

Copy link
Copy Markdown
Contributor

Also, I still prefer @audius/test but its not a hill I'm willing to die on so long as our relative imports don't have infinite ../ (test/test-utils is fine w me)

@audius-infra

Copy link
Copy Markdown
Collaborator

Preview this change https://demo.audius.co/dj-c-4228-component-tests

@dylanjeffers dylanjeffers merged commit 89e52ed into main Apr 3, 2024
@dylanjeffers dylanjeffers deleted the dj-c-4228-component-tests branch April 3, 2024 21:03
audius-infra pushed a commit that referenced this pull request Apr 6, 2024
[6045a76] Fix tracks uploaded without cover art (#8047) Marcus Pasell
[b8ba4f9] Hotfix tracks not getting artwork (#8043) JD Francis
[4bd478a] [PAY-2671] Add purchase button to native mobile playlist feed tiles (#8034) Andrew Mendelsohn
[00a3d79] Fix sale modal link (#8044) Dylan Jeffers
[c5cfed4] [PAY-2675] New Purchase/Sale details modal UI (#8032) Reed
[abd23aa] RTL test setup + SSR store ref refactor (#8042) JD Francis
[ded88f5] [PAY-2624, PAY-2669, PAY-2670] Purchase albums from feed playlist tile (web, mobile web) (#8029) Andrew Mendelsohn
[844db69] RPC fallback on error (#8030) Isaac Solo
[764cfc4] [⚠️][PAY-2583/PAY-2582] Premium album upload flow (#7982) JD Francis
[26968c5] [C-4038] Fix missing playlist image (#8037) Dylan Jeffers
[d1c398d] Upgrade fetch-nft (#8041) Saliou Diallo
[cb205e1] Bump version to 0.6.75 audius-infra
[e70a603] [C-4241] Upgrade dapp-store-cli to latest (#8035) Dylan Jeffers
[2c49141] [C-4058] Refactor InputV2 to harmony TextInput (#7970) Dylan Jeffers
[81e117d] Cache content nodes immediately at startup (#8031) Danny
[0d625ff] [PAY-2654] Use @audius/fetch-nft for all nft fetching logic (#7991) Saliou Diallo
[9e3194b] Use multiple solana rpc connections (#8024) Raymond Jacobson
[2d1c448] Convert DetailsTile mobile component to Harmony (#8017) Reed
[a5beb5d] [C-4329] Fix ai-attribution-details styles (#8021) Dylan Jeffers
[93906a5] [C-4083] Fix native password perf (#8019) Dylan Jeffers
[3dde4f1] [C-4060] Remove Open in Audius App drawer on web mobile for crawlers (#8027) Kyle Shanks
[3fa5684] Bump version to 0.6.74 audius-infra
[4dd65c1] [PAY-2569] Mobile premium album purchase flow (#8006) Reed
[576f265] Slow down solana tx retries (#8023) Isaac Solo
[f3f2cdc] Bump mobile version to 1.1.93 for release (#7988) Dylan Jeffers
[89e52ed] [C-4228] Add component test framework (#8004) Dylan Jeffers
[ad64be9] [INF-593] [INF-644] Clean up npm i output (#7989) Sebastian Klingler
[5a194e7] Fix mobile AI-generated track header (#8015) Reed
[15b8af5] Correct unlisted scheduled releases (#8014) Isaac Solo
[16528bf] Priority listens (#8013) Raymond Jacobson
[ace64e7] Add scheduled release schema type (#8012) Isaac Solo
[512c1c9] ⚠️ Fix and re-enable a subset of tests (PAY-2600, PAY-2601, PAY-2605, PAY-2606, PAY-2607, PAY-2608, PAY-2609, PAY-2610, PAY-2611) (#7961) Marcus Pasell
[0c8d840] Fix mobile collectible svg+xml and video display (#8008) Raymond Jacobson
[01dca30] Fix batch metadata parsing bug and add Fuga test (#8011) Theo Ilie
[d1bd7dc] Add '--maxsockets 1' flag to make ddex build more reliable (#8005) Danny
[6b0901a] Bump version to 0.6.73 audius-infra
[ad513e5] [PROTO-1745] Refactor DDEX parsing: release profiles, Singles, Fuga (#7998) Theo Ilie
[3fe1bd2] Fix useHarmonyField hook dependencies (#8007) Marcus Pasell
[18c3c04] Fix collectible animated webp and progressive loading (#8002) Raymond Jacobson
[b449230] [PAY-2650] Add DetailsTileNoAccess section to collection screen (#8001) Reed
[468d0e9] [ONC-67] Update aggregates counts with more straightforward logic (#7994) Danny
[a8011e8] Block spam nfts (#8003) Saliou Diallo
[9ed0bb4] [C-4233] Turn off client side hydration for crawlers (#8000) Kyle Shanks
[2b36eb1] [C-4235] Fix robots.txt on ssr worker (#7999) Sebastian Klingler
[1bed79d] Fix miscellaneous nft issues (#7995) Saliou Diallo
[43b8cd6] Lower default celery worker concurrency (#7997) Isaac Solo
[d6c0fd7] Bump version to 0.6.72 audius-infra
[782ee51] Add timezone back (#7993) Isaac Solo
[2629b4d] [ONC-68] Fix stream hex signature and delisting by downgrading hexbytes (#7992) Isaac Solo
[299041b] Revert "ONC-54: indexer concurrency (#7933)" (#7990) Isaac Solo
[ffc165c] [PAY-2651] Fix nft video thumbnail (#7986) Saliou Diallo
[b4cab4c] Add textTransform prop to harmony Text (#7984) Reed
[fdb941e] Lint changes for discovery-provider (#7980) Reed
[0245b7a] [C-4225] Add alt text to new StaticImages (#7985) Kyle Shanks
[021ad5e] Fix mobile audio match reward date and collectibles date (#7981) Isaac Solo
[7557f21] [C-4087] Fix library not loading (#7983) Sebastian Klingler
[608aca6] Bump version to 0.6.71 audius-infra
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants