Skip to content

Commit

Permalink
Fix unit test warnings (#5172)
Browse files Browse the repository at this point in the history
  • Loading branch information
obulat authored Nov 25, 2024
1 parent ceed798 commit 380eb31
Show file tree
Hide file tree
Showing 23 changed files with 69 additions and 246 deletions.
2 changes: 0 additions & 2 deletions eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,6 @@ const eslintIgnores = [
"frontend/.output/**",
"frontend/.remake/**",
"frontend/src/locales/*.json",
// Vendored module. See explanation in file
"frontend/test/unit/test-utils/render-suspended.ts",
"**/coverage/**",
"frontend/test/tapes/**",
"frontend/storybook-static/**",
Expand Down
5 changes: 5 additions & 0 deletions frontend/src/components/VButton.vue
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,10 @@ const disabledAttribute = computed<true | undefined>(() => {
return (trulyDisabled && supportsDisabledAttribute.value) || undefined
})
const linkProps = computed(() =>
props.as === "VLink" ? { href: attrs.href } : {}
)
watch(
() => props.as,
(as) => {
Expand Down Expand Up @@ -229,6 +233,7 @@ watch(
:aria-pressed="pressed"
:aria-disabled="ariaDisabled"
:disabled="disabledAttribute"
v-bind="linkProps"
@click="$emit('click', $event)"
@mousedown="$emit('mousedown', $event)"
@keydown="$emit('keydown', $event)"
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/components/VHeader/VHomeLink.vue
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ withDefaults(
*
* @default 'light'
*/
variant: "light" | "dark"
variant?: "light" | "dark"
}>(),
{ variant: "light" }
)
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/components/VHeader/VWordPressLink.vue
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import VSvg from "~/components/VSvg/VSvg.vue"
withDefaults(
defineProps<{
mode: "dark" | "light"
mode?: "dark" | "light"
}>(),
{ mode: "light" }
)
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/components/VLogoLoader/VLogoLoader.vue
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { useReducedMotion } from "~/composables/use-reduced-motion"
withDefaults(
defineProps<{
status: "loading" | "idle"
status?: "loading" | "idle"
}>(),
{
status: "idle",
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/components/VTag/VTag.vue
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<script setup lang="ts">
import VButton from "~/components/VButton.vue"
defineProps<{ href: string }>()
const props = defineProps<{ href: string }>()
</script>

<template>
Expand All @@ -10,7 +10,7 @@ defineProps<{ href: string }>()
size="small"
variant="filled-gray"
class="label-bold"
v-bind="$props"
v-bind="props"
><slot
/></VButton>
</template>
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
import { fireEvent } from "@testing-library/vue"

import { createApp } from "vue"
import { createApp, nextTick } from "vue"

import { render } from "~~/test/unit/test-utils/render"

import { i18n } from "~~/test/unit/test-utils/i18n"
import { getAudioObj } from "~~/test/unit/fixtures/audio"

import { useActiveMediaStore } from "~/stores/active-media"
Expand All @@ -24,17 +22,31 @@ const RouterLinkStub = createApp({}).component("RouterLink", {
},
},
})._context.components.RouterLink

const stubs = {
VLicense: true,
VWaveform: true,
VAudioThumbnail: true,
RouterLink: RouterLinkStub,
}

const captureExceptionMock = vi.fn()

vi.mock("#app", async () => {
const original = await import("#app")
return {
...original,
useNuxtApp: vi.fn(() => ({
$sentry: {
captureException: captureExceptionMock,
},
})),
}
})

describe("AudioTrack", () => {
let options = null
let props = null
const captureExceptionMock = vi.fn()

beforeEach(() => {
props = {
Expand All @@ -54,7 +66,6 @@ describe("AudioTrack", () => {
options = {
props: props,
global: {
plugins: [i18n],
stubs,
},
}
Expand Down Expand Up @@ -94,8 +105,7 @@ describe("AudioTrack", () => {
expect(creator).toBeTruthy()
})

// https://github.com/wordpress/openverse/issues/411
it.skip.each`
it.each`
errorType | errorText
${"NotAllowedError"} | ${/Reproduction not allowed./i}
${"NotSupportedError"} | ${/This audio format is not supported by your browser./i}
Expand All @@ -111,19 +121,22 @@ describe("AudioTrack", () => {

vi.clearAllMocks()

const pauseStub = vi
.spyOn(window.HTMLMediaElement.prototype, "pause")
.mockImplementation(() => undefined)

const pauseStub = vi.fn(() => undefined)
const playStub = vi.fn(() => Promise.reject(playError))
const playError = new DOMException("msg", errorType)

const playStub = vi
.spyOn(window.HTMLMediaElement.prototype, "play")
.mockImplementation(() => Promise.reject(playError))
vi.spyOn(window.HTMLMediaElement.prototype, "pause").mockImplementation(
pauseStub
)

vi.spyOn(window.HTMLMediaElement.prototype, "play").mockImplementation(
playStub
)

const { getByRole, getByText } = await render(VAudioTrack, options)

await fireEvent.click(getByRole("button"))
await fireEvent.click(getByRole("button", { name: /play/i }))
await nextTick()
expect(playStub).toHaveBeenCalledTimes(1)
expect(pauseStub).toHaveBeenCalledTimes(1)
expect(getByText(errorText)).toBeVisible()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ describe("VCopyLicense", () => {
creator_url: "http://creator.com",
frontendMediaType: "image",
},
fullLicenseName: "LICENSE",
}
options = { props }
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import VInputField from "~/components/VInputField/VInputField.vue"
const props = {
fieldId: "input-id",
labelText: "Label",
size: "medium",
}

describe("VInputField", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ describe("VMediaDetails", () => {
}
})

// https://github.com/wordpress/openverse/issues/411
// https://github.com/wordpress/openverse/issues/5171
it.skip("renders the album title", async () => {
await render(VMediaDetails, options)

Expand All @@ -39,27 +39,27 @@ describe("VMediaDetails", () => {
)
})

// https://github.com/wordpress/openverse/issues/411
// https://github.com/wordpress/openverse/issues/5171
it.skip("hides the album title tag when it does not exists", async () => {
options.props.media.audio_set = null
await render(VMediaDetails, options)
expect(screen.queryByText("Album")).toBeNull()
})

// https://github.com/wordpress/openverse/issues/411
// https://github.com/wordpress/openverse/issues/5171
it.skip("displays the main filetype when no alternative files are available", async () => {
await render(VMediaDetails, options)
expect(screen.queryByText("MP32")).toBeVisible()
})

// https://github.com/wordpress/openverse/issues/411
// https://github.com/wordpress/openverse/issues/5171
it.skip("displays multiple filetypes when they are available in alt_files", async () => {
options.props.media.alt_files = [{ filetype: "wav" }, { filetype: "ogg" }]
await render(VMediaDetails, options)
expect(screen.queryByText("MP32, WAV, OGG")).toBeVisible()
})

// https://github.com/wordpress/openverse/issues/411
// https://github.com/wordpress/openverse/issues/5171
it.skip("displays only distinct filetypes", async () => {
options.props.media.alt_files = [{ filetype: "ogg" }, { filetype: "ogg" }]
await render(VMediaDetails, options)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import { createApp } from "vue"

import { render } from "~~/test/unit/test-utils/render"

import { i18n } from "~~/test/unit/test-utils/i18n"

import { useSearchStore } from "~/stores/search"

import VSafetyWall from "~/components/VSafetyWall/VSafetyWall.vue"
Expand All @@ -25,17 +23,15 @@ describe("VSafetyWall.vue", () => {
beforeEach(() => {
options = {
global: {
plugins: [i18n],
stubs: { RouterLink: RouterLinkStub },
},
props: {
media: {
sensitivity: [
"sensitive_text",
"provider_supplied_sensitive",
"user_reported_sensitive",
],
},
sensitivity: [
"sensitive_text",
"provider_supplied_sensitive",
"user_reported_sensitive",
],
id: "123",
},
}
})
Expand Down
7 changes: 4 additions & 3 deletions frontend/test/unit/specs/components/VTag/v-tag.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,11 @@ describe("VTag", () => {
expect(link.href).toEqual("https://example.com/")
})

// https://github.com/wordpress/openverse/issues/411
it.skip("renders slot content", async () => {
it("renders slot content", async () => {
const slotText = "Slot test"
options.slots = { default: () => `<div>${slotText}</div>` }
// Using a non-function value for the slot causes a Vue warning,
// but a function value is not rendered correctly by the unit tests.
options.slots = { default: `<div>${slotText}</div>` }

await render(VTag, options)
expect(screen.getByText(slotText)).toBeDefined()
Expand Down
6 changes: 1 addition & 5 deletions frontend/test/unit/specs/components/scroll-button.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,11 @@ import { screen } from "@testing-library/vue"

import { render } from "~~/test/unit/test-utils/render"

import { i18n } from "~~/test/unit/test-utils/i18n"

import VScrollButton from "~/components/VScrollButton.vue"

describe("Scroll button", () => {
it("should render a scroll button", async () => {
const { container } = await render(VScrollButton, {
global: { plugins: [i18n] },
})
const { container } = await render(VScrollButton)
expect(screen.getByRole("button")).toBeTruthy()
expect(screen.getByLabelText(/scroll/i)).toBeTruthy()
expect(container.querySelectorAll("svg").length).toEqual(1)
Expand Down
3 changes: 0 additions & 3 deletions frontend/test/unit/specs/components/v-content-link.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ import { createApp } from "vue"

import { render } from "~~/test/unit/test-utils/render"

import { i18n } from "~~/test/unit/test-utils/i18n"

import VContentLink from "~/components/VContentLink/VContentLink.vue"

const RouterLinkStub = createApp({}).component("RouterLink", {
Expand All @@ -26,7 +24,6 @@ describe("VContentLink", () => {
beforeEach(() => {
options = {
global: {
plugins: [i18n],
stubs: { RouterLink: RouterLinkStub },
},
props: {
Expand Down
3 changes: 1 addition & 2 deletions frontend/test/unit/specs/components/v-image-cell.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { createApp } from "vue"

import { image } from "~~/test/unit/fixtures/image"
import { render } from "~~/test/unit/test-utils/render"
import { i18n } from "~~/test/unit/test-utils/i18n"

import VImageCell from "~/components/VImageCell/VImageCell.vue"

Expand All @@ -21,13 +20,13 @@ describe("VImageCell", () => {
beforeEach(() => {
options = {
global: {
plugins: [i18n],
stubs: {
RouterLink: RouterLinkStub,
},
},
props: {
image,
kind: "search",
searchTerm: "cat",
relatedTo: null,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { getApiAccessToken } from "~/plugins/01.api-token.server"

describe("missing client credentials", () => {
describe("completely missing", () => {
// https://github.com/wordpress/openverse/issues/411
// https://github.com/wordpress/openverse/issues/5171
it.skip("should not make any requests and fall back to tokenless", async () => {
const token = await getApiAccessToken()

Expand All @@ -13,7 +13,7 @@ describe("missing client credentials", () => {
})

describe("explicitly undefined", () => {
// https://github.com/wordpress/openverse/issues/411
// https://github.com/wordpress/openverse/issues/5171
it.skip("should not make any requests and fall back to tokenless", async () => {
const accessToken = await getApiAccessToken({
apiClientId: undefined,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ vi.mock("#app/nuxt", async () => {
}
})

// https://github.com/wordpress/openverse/issues/411
// https://github.com/wordpress/openverse/issues/5171
it.skip("subsequent requests should all block on the same token retrieval promise", async () => {
/**
* This test is pretty complicated because we need to simulate
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ describe.sequential("api-token", () => {
})

describe("successful token retrieval", () => {
// https://github.com/wordpress/openverse/issues/411
// https://github.com/wordpress/openverse/issues/5171
it.skip("should save the token into the process and inject into the context", async () => {
const mockTokenResponse = getMockTokenResponse()
axios.post.mockImplementationOnce(() =>
Expand All @@ -67,7 +67,7 @@ describe.sequential("api-token", () => {
})
})

// https://github.com/wordpress/openverse/issues/411
// https://github.com/wordpress/openverse/issues/5171
it.skip("should re-retrieve the token when about to expire", async () => {
const mockTokenResponse = getMockTokenResponse(expiryThreshold - 1)
const nextMockTokenResponse = getMockTokenResponse()
Expand All @@ -89,7 +89,7 @@ describe.sequential("api-token", () => {
})
})

// https://github.com/wordpress/openverse/issues/411
// https://github.com/wordpress/openverse/issues/5171
it.skip("should not request a new token if the token is not about to expire", async () => {
const mockTokenResponse = getMockTokenResponse(twelveHoursInSeconds)
const nextMockTokenResponse = getMockTokenResponse()
Expand Down
Loading

0 comments on commit 380eb31

Please sign in to comment.