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.

Sign up for GitHub

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

Configure vitest with API and Pinia mocking #229

Merged
merged 10 commits into from
Sep 26, 2023
Merged

Configure vitest with API and Pinia mocking #229

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
f50d128
Install @vue/test-utils
dotNomad Sep 20, 2023
b9e87ca
Install jsdom
dotNomad Sep 21, 2023
f5559b5
Set vitest testing env to jsdom
dotNomad Sep 21, 2023
09af4fc
Include unit test .ts files in tsconfig
dotNomad Sep 21, 2023
7fdd5d6
Install @pinia/testing
dotNomad Sep 21, 2023
0636b5f
Enable the Dark plugin for Quasar
dotNomad Sep 21, 2023
9a012d8
Remove DeploymentFile.root as its unused
dotNomad Sep 25, 2023
48be4cd
Export all enums in api/index.ts
dotNomad Sep 25, 2023
ea2c99e
Create vitest with mocked api and pinia
dotNomad Sep 25, 2023
a2ed261
Merge branch 'main' into dotnomad/vitest
dotNomad Sep 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
947 changes: 946 additions & 1 deletion web/package-lock.json
View file
Open in desktop

Large diffs are not rendered by default.

3 changes: 3 additions & 0 deletions web/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,18 @@
"vue": "^3.3.4"
},
"devDependencies": {
"@pinia/testing": "^0.1.3",
"@quasar/vite-plugin": "^1.4.1",
"@types/node": "^20.5.4",
"@typescript-eslint/eslint-plugin": "^6.4.1",
"@typescript-eslint/parser": "^6.4.1",
"@vitejs/plugin-vue": "^4.2.3",
"@vue/test-utils": "^2.4.1",
"cypress": "^13.0.0",
"eslint": "^8.47.0",
"eslint-plugin-cypress": "^2.14.0",
"eslint-plugin-vue": "^9.17.0",
"jsdom": "^22.1.0",
"sass": "^1.66.1",
"start-server-and-test": "^2.0.0",
"typescript": "^5.0.2",
Expand Down
12 changes: 6 additions & 6 deletions web/src/api/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

export { api as default, useApi } from 'src/api/client';

export type * from 'src/api/types/accounts';
export type * from 'src/api/types/apptypes';
export type * from 'src/api/types/connect';
export type * from 'src/api/types/deployments';
export type * from 'src/api/types/files';
export type * from 'src/api/types/manifest';
export * from 'src/api/types/accounts';
export * from 'src/api/types/apptypes';
export * from 'src/api/types/connect';
export * from 'src/api/types/deployments';
export * from 'src/api/types/files';
export * from 'src/api/types/manifest';
1 change: 0 additions & 1 deletion web/src/api/types/files.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ export type DeploymentFile = {
abs: string
base: string
rel: string
root: string
size: number
modifiedDatetime: string
isDir: boolean
Expand Down
4 changes: 2 additions & 2 deletions web/src/main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import { createApp } from 'vue';
import { createPinia } from 'pinia';
import { Quasar } from 'quasar';
import { Dark, Quasar } from 'quasar';

// Import Quasar Roboto Font
import '@quasar/extras/roboto-font/roboto-font.css';
Expand All @@ -20,5 +20,5 @@ const pinia = createPinia();
const app = createApp(App);

app.use(pinia);
app.use(Quasar, {});
app.use(Quasar, { plugins: { Dark } });
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This plugin had to be enabled to avoid an error. I couldn't find a way around this, or what caused it. Perhaps using the dark prop uses the Dark plugin behind the scenes. This wasn't documented, but components would not mount using Vue Test Utils without enabling this since $q.dark's properties were accessed.

app.mount('#app');
203 changes: 203 additions & 0 deletions web/tests/unit/components/configurePublish/FilesToPublish.test.ts
View file
Open in desktop
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The mock response data and store data here is very large. My suggestion would be to, in future work, break these out into another file to quickly import with a helpful name.

Original file line number Diff line number Diff line change
@@ -0,0 +1,203 @@
import { Quasar } from 'quasar';
import { describe, expect, test, vi } from 'vitest';
import { createTestingPinia } from '@pinia/testing';
import { flushPromises, mount } from '@vue/test-utils';

import api, { AppMode, DeploymentFile, DeploymentFileType, ExclusionMatchSource, ServerType, useApi } from 'src/api';
import FilesToPublish from 'src/components/configurePublish/FilesToPublish.vue';
import { useDeploymentStore } from 'src/stores/deployment';

const fakeResponse: DeploymentFile = {
id: '.',
fileType: DeploymentFileType.DIRECTORY,
base: 'fastapi-simple',
exclusion: null,
files: [
{
id: '.gitignore',
fileType: DeploymentFileType.REGULAR,
base: '.gitignore',
exclusion: {
source: ExclusionMatchSource.BUILT_IN,
pattern: '.gitignore',
filePath: '',
line: 0
},
files: [],
isDir: false,
isEntrypoint: false,
isFile: true,
modifiedDatetime: '2023-09-19T10:20:06-07:00',
rel: '.gitignore',
size: 6,
abs: '/Users/jordan/development/publishing-client/test/sample-content/fastapi-simple/.gitignore'
},
{
id: 'manifest.json',
fileType: DeploymentFileType.REGULAR,
base: 'manifest.json',
exclusion: {
source: ExclusionMatchSource.BUILT_IN,
pattern: 'manifest.json',
filePath: '',
line: 0
},
files: [],
isDir: false,
isEntrypoint: false,
isFile: true,
modifiedDatetime: '2023-08-18T11:56:15-07:00',
rel: 'manifest.json',
size: 552,
abs: '/Users/jordan/development/publishing-client/test/sample-content/fastapi-simple/manifest.json'
},
{
id: 'meta.yaml',
fileType: DeploymentFileType.REGULAR,
base: 'meta.yaml',
exclusion: null,
files: [],
isDir: false,
isEntrypoint: false,
isFile: true,
modifiedDatetime: '2023-08-02T15:41:24-07:00',
rel: 'meta.yaml',
size: 63,
abs: '/Users/jordan/development/publishing-client/test/sample-content/fastapi-simple/meta.yaml'
},
{
id: 'requirements.in',
fileType: DeploymentFileType.REGULAR,
base: 'requirements.in',
exclusion: null,
files: [],
isDir: false,
isEntrypoint: false,
isFile: true,
modifiedDatetime: '2023-08-02T15:41:24-07:00',
rel: 'requirements.in',
size: 124,
abs: '/Users/jordan/development/publishing-client/test/sample-content/fastapi-simple/requirements.in'
},
{
id: 'requirements.txt',
fileType: DeploymentFileType.REGULAR,
base: 'requirements.txt',
exclusion: null,
files: [],
isDir: false,
isEntrypoint: false,
isFile: true,
modifiedDatetime: '2023-08-02T15:41:24-07:00',
rel: 'requirements.txt',
size: 235,
abs: '/Users/jordan/development/publishing-client/test/sample-content/fastapi-simple/requirements.txt'
},
{
id: 'simple.py',
fileType: DeploymentFileType.REGULAR,
base: 'simple.py',
exclusion: null,
files: [],
isDir: false,
isEntrypoint: false,
isFile: true,
modifiedDatetime: '2023-08-02T15:41:24-07:00',
rel: 'simple.py',
size: 369,
abs: '/Users/jordan/development/publishing-client/test/sample-content/fastapi-simple/simple.py'
}
],
isDir: true,
isEntrypoint: false,
isFile: false,
modifiedDatetime: '2023-09-19T10:20:06-07:00',
rel: '.',
size: 256,
abs: '/Users/jordan/development/publishing-client/test/sample-content/fastapi-simple'
};

vi.mock('src/api');
vi.mocked(useApi).mockReturnValue(api);
Comment on lines +120 to +121
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This bit was extremely tricky. vi.mock('src/api') with no manual factory set will:

Vitest will mock the module itself by invoking it and mocking every export.
The following principles apply

  • All arrays will be emptied
  • All primitives and collections will stay the same
  • All objects will be deeply cloned
  • All instances of classes and their prototypes will be deeply cloned

So great we get a nice mock with no effort. However our API module has two exports that rely on each other:

export const api = new PublishingClientApi();

export const useApi = () => api;

and Vitest has no clue about this when it constructs its mock.

So api correctly gets setup as a new, mocked PublishingClientApi but useApi gets mocked as a vi.fn with no return.

vi.mocked(useApi).mockReturnValue(api);

Is here to recreate that relationship so both api can be mocked here, and in components useApi() will return the mocked PublishingClientApi class instance.

This can probbably be broken out into a __mock__ file at some point to avoid the boilerplate, but its so small that that felt like a future task.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's a funny call when separating out the test data. I think it would be useful to have a way of having base test data which could be pulled in from an external file, but then also overlay specific test data changes on top of it, for a particular test. This can obviously be created organically as we go forward, but it would be good to outline a small pattern to use so we could all be consistent.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Completely agree. Having test data in an external file that we can override partially would be perfect. I think another PR introducing that pattern with another test would be a great next step.


describe('description', () => {
test('some test description', async() => {
vi.mocked(api.files.get, { partial: true }).mockResolvedValue({ data: fakeResponse });
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also a bit tricky. Once the mocking was fixed this became pretty easy! The partial: true allows us to avoid needing to fully type the AxiosResponse object instead only setting the thing(s) we need (data in this case).


const wrapper = mount(FilesToPublish, {
global: {
plugins: [Quasar, createTestingPinia({ createSpy: vi.fn })]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Each time we mount we need to enable the Quasar plugin, and create a testing Pinia instance (if we need Pinia for the test(s)). Since we don't use globals we need to create a spy ourselves which is very straightforward.

}
});

const deploymentStore = useDeploymentStore();
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems a bit strange to me that we call this after we mount the component. I would think of this as part of the test setup, all available ahead of the component being mounted (at which time it would run through all of its normal behavior).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree. The Pinia docs don't explicitly say it, but in their examples always have the use store code under the mount 1. If we move it above we get an error:

 FAIL  tests/unit/components/configurePublish/FilesToPublish.test.ts > description > some test description
Error: [🍍]: "getActivePinia()" was called but there was no active Pinia. Did you forget to install pinia?
	const pinia = createPinia()
	app.use(pinia)
This will fail in production.

Since the mount is actually creating the Pinia we need to do it first. I actually had the same reaction and tried moving it above when things weren't quite working wondering if that was a fix.

Footnotes

  1. https://pinia.vuejs.org/cookbook/testing.html#Unit-testing-components ↩

deploymentStore.deployment = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

With the deployment store being so "mono" across all of our settings, we're definitely going to want to put something better in place so we're not always copying/pasting the state over and over.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

100% agree. I think it will look very similar to your proposal of importing things from an external file and overriding where necessary. In a follow-up PR introducing this will be important as soon as we have more than a single test.

sourcePath: 'test/sample-content/fastapi-simple',
target: {
accountName: 'dogfood',
serverType: ServerType.CONNECT,
serverUrl: '',
contentId: '',
contentName: '',
username: '',
bundleId: null,
deployedAt: null
},
manifest: {
version: 1,
locale: '',
metadata: {
appmode: AppMode.PYTHON_FASTAPI,
entrypoint: 'simple.py',
primaryHtml: '',
},
python: {
version: '3.11.5',
packageManager: {
name: 'pip',
packageFile: 'requirements.txt'
}
},
packages: {},
files: {
'.': { checksum: '' },
'meta.yaml': { checksum: '' },
'requirements.in': { checksum: '' },
'requirements.txt': { checksum: '' },
'simple.py': { checksum: '' }
}
},
connect: {
connect: {
name: '',
connectionTimeout: null,
readTimeout: null,
initTimeout: null,
idleTimeout: null,
maxProcesses: null,
minProcesses: null,
maxConnsPerProcess: null,
loadFactor: null,
runAsCurrentUser: null,
memoryRequest: null,
memoryLimit: null,
cpuRequest: null,
cpuLimit: null
},
environment: [
{
name: '',
value: '',
fromEnvironment: false,
}
]
},
pythonRequirements: ['YW55aW89PTMuNi4yCmFzZ2lyZWY9PTMuNi4wCmNsaWNrPT04LjEuMwpmYXN0YXBpPT0wLjk1LjIKaDExPT0wLjE0LjAKaWRuYT09My40CnB5ZGFudGljPT0xLjEwLjcKcHlqd3Q9PTIuNy4wCnJzY29ubmVjdC1weXRob249PTEuMTcuMApzZW12ZXI9PTIuMTMuMApzaXg9PTEuMTYuMApzbmlmZmlvPT0xLjMuMApzdGFybGV0dGU9PTAuMjcuMAp0eXBpbmctZXh0ZW5zaW9ucz09NC41LjAKdXZpY29ybj09MC4yMi4wCg==']
};
await flushPromises();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was also extremely tricky. Without this our tests execute before the component has the opportunity to re-render after the deploymentStore.deployment is set.

So we need to mount the component to get the Pinia store, but we need to wait until that is all done before we move on. One way to do this is with flushPromises which will resolve all promise handles and ensure async operations and DOM updates happen before our expects.

The docs for Vue Test Utils also recommend using nextTick to wait for the DOM, but this doesn't work. I assume this is because we are waiting for some async code in Pinia, and not ONLY the Vue tick to occur for updating.

References:


expect(api.files.get).toHaveBeenCalledOnce();
expect(wrapper.text()).toContain('4 files selected from test/sample-content/fastapi-simple (total = 1.0 KB)');
});
});

8 changes: 7 additions & 1 deletion web/tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@
"src/*": ["src/*"]
}
},
"include": ["src/**/*.ts", "src/**/*.d.ts", "src/**/*.tsx", "src/**/*.vue"],
"include": [
"src/**/*.ts",
"src/**/*.d.ts",
"src/**/*.tsx",
"src/**/*.vue",
"tests/**/*.ts"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Needed to add our test files to ensure they got our TypeScript settings and the path alias.

],
"references": [{ "path": "./tsconfig.node.json" }]
}
5 changes: 5 additions & 0 deletions web/vite.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
/// <reference types="vitest" />
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is needed at the top to expand the config type to include test to set the Vitest testing environment to jsdom.


import { fileURLToPath } from 'node:url';
import { defineConfig } from 'vite';
import vue from '@vitejs/plugin-vue';
Expand Down Expand Up @@ -31,5 +33,8 @@ export default defineConfig({
secure: false
}
}
},
test: {
environment: 'jsdom',
}
});