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.

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

Add test coverage where missing #15

Merged
merged 29 commits into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
853cf4f
Add coverage for api/run/results
chrisbendel Dec 3, 2024
777c33c
More progress, single directory causing flakiness
chrisbendel Dec 3, 2024
2c77f68
Disable parallelism for consistency
chrisbendel Dec 4, 2024
85150d6
dont include tests in coverage report
chrisbendel Dec 4, 2024
8b9b2a4
Add mantine render for tests
chrisbendel Dec 4, 2024
1ccc40c
Cleanup, notes
chrisbendel Dec 5, 2024
8867a07
Lint
chrisbendel Dec 9, 2024
7057cb0
Comments
chrisbendel Dec 9, 2024
54d3eab
Save work
chrisbendel Dec 9, 2024
0580385
Use js dom for dom testing?
chrisbendel Dec 9, 2024
23ec05c
middleware
chrisbendel Dec 10, 2024
2ac99cd
Getting there...
chrisbendel Dec 11, 2024
5109aee
96% coverage in providers, UGH
chrisbendel Dec 11, 2024
ccf1b07
Check if data for a run already exists on POST
rnathuji Dec 11, 2024
bc86dee
Ensure runId values are UUIDs
rnathuji Dec 11, 2024
d87b3bc
Ensure upload data includes only expected file key
rnathuji Dec 11, 2024
7757f4c
Fix lint issue
rnathuji Dec 12, 2024
2325b36
Merge pull request #16 from safeinsights/SHRMP-52/error-handling
rnathuji Dec 12, 2024
3819541
Update providers
chrisbendel Dec 12, 2024
1cfb3cd
Lint
chrisbendel Dec 12, 2024
a69158e
Type safe imports?
chrisbendel Dec 12, 2024
8cb1485
Typechekcs
chrisbendel Dec 12, 2024
839fa35
Remove unused layout tests for now
chrisbendel Dec 12, 2024
1fbab7e
Update providers to get closer and less flaky
chrisbendel Dec 12, 2024
e6f4f86
providers again
chrisbendel Dec 13, 2024
122e2ab
Ignore providers.test.tsx for now temporarily
chrisbendel Dec 13, 2024
4b23c22
Make CI happy?
chrisbendel Dec 13, 2024
e3e56b0
Use uuid for tests, updates per code review
chrisbendel Dec 18, 2024
46704e8
Get things working
chrisbendel Dec 18, 2024
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
459 changes: 449 additions & 10 deletions package-lock.json

Large diffs are not rendered by default.

7 changes: 6 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@
"license": "AGPL-3.0-or-later",
"devDependencies": {
"@playwright/test": "^1.46",
"@tanstack/query-core": "^5.62.3",
"@testing-library/react": "^16.0",
"@types/jsonwebtoken": "^9.0.7",
"@types/mock-fs": "^4.13.4",
"@types/react": "18.3",
"@vanilla-extract/vite-plugin": "^4.0.13",
"@vitejs/plugin-react": "^4.3.1",
Expand All @@ -35,6 +37,8 @@
"eslint-config-next": "*",
"eslint-config-prettier": "*",
"happy-dom": ">=15.10.2",
"jsdom": "^25.0.1",
"mock-fs": "^5.4.1",
"npm-run-all": "*",
"prettier": "*",
"vite-tsconfig-paths": "^5.0.1",
Expand All @@ -54,6 +58,7 @@
"jsonwebtoken": "^9.0.2",
"mantine-datatable": "^7.12.4",
"multer": "^1.4.5-lts.1",
"next-swagger-doc": "^0.4"
"next-swagger-doc": "^0.4",
"uuid": "^11.0.3"
}
}
15 changes: 15 additions & 0 deletions src/app/api/health/route.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { GET } from './route'
import { describe, it, expect } from 'vitest'

describe('Healthcheck Endpoint', () => {
it('should return the correct healthcheck response', async () => {
const response = await GET()
const data = await response.json()

expect(response.status).toBe(200)
expect(data).toEqual({
success: true,
message: { status: 'ok' },
})
})
})
2 changes: 1 addition & 1 deletion src/app/api/health/route.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { NextResponse } from 'next/server'
export const GET = async () => {
return NextResponse.json({
success: false,
success: true,
message: { status: 'ok' },
})
}
75 changes: 75 additions & 0 deletions src/app/api/run/[runId]/approve/route.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
import { POST } from '@/app/api/run/[runId]/approve/route'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { UPLOAD_DIR } from '@/app/utils'
import mockFs from 'mock-fs'
import { v4 } from 'uuid'

describe('POST /api/run/:runId/approve', () => {
const mockFileContent = 'header1,header2\nvalue1,value2'
const runId = v4()

beforeEach(() => {
// Mock the file system with the necessary file
mockFs({
[UPLOAD_DIR]: {
[runId]: mockFileContent,
},
})
})

it('should return 400 if runId is missing', async () => {
const req = new Request('http://localhost', { method: 'POST' })
// @ts-ignore
const res = await POST(req as any, { params: {} })

// Check status
expect(res.status).toBe(400)
})

it('should return 400 if the file does not exist', async () => {
// Simulate the file not existing by removing it from mockFs
mockFs({
[UPLOAD_DIR]: {}, // Empty the directory to simulate no file for 'runId'
})

const req = new Request('http://localhost', { method: 'POST' })
const res = await POST(req as any, { params: { runId: 'non-existent-run' } })

// Check status
expect(res.status).toBe(400)

// Check JSON payload
const data = await res.json()
expect(data).toEqual({ error: 'No file exists to delete' })
})

it('should return 500 if the external API request fails', async () => {
// Mock the fetch call to simulate failure
vi.stubGlobal('fetch', vi.fn().mockResolvedValueOnce({ ok: false }))

const req = new Request('http://localhost', { method: 'POST' })
const res = await POST(req as any, { params: { runId } })

// Check status
expect(res.status).toBe(500)

// Check JSON payload
const data = await res.json()
expect(data).toEqual({ error: 'Unable to post file' })
})

it('should return 200 and success if the file exists and external API request succeeds', async () => {
// Mock the fetch call to simulate success
vi.stubGlobal('fetch', vi.fn().mockResolvedValueOnce({ ok: true }))

const req = new Request('http://localhost', { method: 'POST' })
const res = await POST(req as any, { params: { runId } })

// Check status
expect(res.status).toBe(200)

// Check JSON payload
const data = await res.json()
expect(data).toEqual({ success: true })
})
})
5 changes: 0 additions & 5 deletions src/app/api/run/[runId]/approve/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,6 @@ import { deleteFile, generateAuthorizationHeaders, UPLOAD_DIR } from '@/app/util
import path from 'path'
import fs from 'fs'

// TODO This component will do two things
// 1. Post the file to the management API
// 2. If the management call is successful, we will delete the file from our filesystem
// 2.a If unsuccessful - show appropriate error message?
// To be done in https://openstax.atlassian.net/browse/SHRMP-21
export const POST = async (req: NextRequest, { params }: { params: { runId: string } }) => {
const runId = params.runId

Expand Down
97 changes: 78 additions & 19 deletions src/app/api/run/[runId]/upload/route.test.ts
Original file line number Diff line number Diff line change
@@ -1,34 +1,24 @@
import { afterEach, beforeEach, describe, expect, it } from 'vitest'
import { POST } from '@/app/api/run/[runId]/upload/route'
import { describe, expect, it, beforeEach } from 'vitest'
import { POST } from './route'
import { NextRequest } from 'next/server'
import path from 'path'
import fs from 'fs'
import { rm } from 'fs/promises'
import { UPLOAD_DIR } from '@/app/utils' // for cleanup
import { UPLOAD_DIR } from '@/app/utils'
import mockFs from 'mock-fs'
import { v4 as uuidv4 } from 'uuid'

// Ensure the upload directory exists
beforeEach(() => {
if (!fs.existsSync(UPLOAD_DIR)) {
fs.mkdirSync(UPLOAD_DIR, { recursive: true })
}
})

// Clean up after each test
afterEach(async () => {
await rm(UPLOAD_DIR, { recursive: true, force: true })
fs.rmSync(UPLOAD_DIR, { recursive: true, force: true })
})

describe('POST /api/run/[runId]/upload', () => {
it('should upload a file successfully', async () => {
// Mock a CSV file as a Blob
const mockFile = new Blob(['id,name\n1,John'], { type: 'text/csv' })
const mockRunId = '123'
const mockRunId = uuidv4()

// Create a FormData object with the mock file
const formData = new FormData()
formData.append('file', new File([mockFile], mockRunId))

// Mock the NextRequest
const req = {
formData: async () => formData,
} as NextRequest
Expand All @@ -48,15 +38,84 @@ describe('POST /api/run/[runId]/upload', () => {
})

it('should return failure if no file is uploaded', async () => {
// Empty FormData object
const formData = new FormData()

// Mock the NextRequest
const req = {
formData: async () => formData,
} as NextRequest
const params = { runId: '123' }
const params = { runId: uuidv4() }
const response = await POST(req, { params })
expect(response.status).toBe(400)
expect((await response.json()).error).toBe('Form data does not include expected file key')
})

it('should return failure if unexpected form data is included', async () => {
const mockFile = new Blob(['id,name\n1,John'], { type: 'text/csv' })
const mockRunId = uuidv4()

const formData = new FormData()
formData.append('file', new File([mockFile], mockRunId))
formData.append('file2', new File([mockFile], mockRunId))

const req = {
formData: async () => formData,
} as NextRequest
const params = { runId: uuidv4() }
const response = await POST(req, { params })
expect(response.status).toBe(400)
expect((await response.json()).error).toBe('Form data includes unexpected data keys')
})

it('should return failure if runId is not a UUID', async () => {
const mockRunId = '123'

const formData = new FormData()

const req = {
formData: async () => formData,
} as NextRequest

const params = { runId: mockRunId }

const response = await POST(req, { params })
expect(response.status).toBe(400)
expect((await response.json()).error).toBe('runId is not a UUID')
})

it('should return an error if no runID is provided', async () => {
const formData = new FormData()

// Mock the NextRequest
const req = {
formData: async () => formData,
} as NextRequest
const params = {}
// @ts-ignore
const response = await POST(req, { params })
expect(response.status).toBe(400)
})

it('should return an error if the runID has results already', async () => {
const mockRunId = uuidv4()

mockFs({
[UPLOAD_DIR]: {
[mockRunId]: '',
},
})

const formData = new FormData()
formData.append('file', '')

const req = {
formData: async () => formData,
} as NextRequest

const params = { runId: mockRunId }

const response = await POST(req, { params })
expect(response.status).toBe(400)
expect((await response.json()).error).toBe('Data already exists for runId')
})
})
21 changes: 20 additions & 1 deletion src/app/api/run/[runId]/upload/route.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { NextRequest, NextResponse } from 'next/server'
import { saveFile } from '@/app/utils'
import { saveFile, UPLOAD_DIR, isValidUUID } from '@/app/utils'
import path from 'path'
import fs from 'fs'

function isFile(obj: any): obj is File {
return obj instanceof File
Expand All @@ -14,6 +16,23 @@ export const POST = async (req: NextRequest, { params }: { params: { runId: stri
return NextResponse.json({ error: 'Missing runId' }, { status: 400 })
}

if (!isValidUUID(runId)) {
return NextResponse.json({ error: 'runId is not a UUID' }, { status: 400 })
}

if (!('file' in body)) {
return NextResponse.json({ error: 'Form data does not include expected file key' }, { status: 400 })
}

if (Object.keys(body).length !== 1) {
return NextResponse.json({ error: 'Form data includes unexpected data keys' }, { status: 400 })
}

const filePath = path.join(UPLOAD_DIR, runId)
if (fs.existsSync(filePath)) {
return NextResponse.json({ error: 'Data already exists for runId' }, { status: 400 })
}

if ('file' in body && isFile(body.file)) {
await saveFile(body.file, runId)
return NextResponse.json({}, { status: 200 })
Expand Down
42 changes: 42 additions & 0 deletions src/app/api/run/results/route.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import { GET } from './route'
import { beforeEach, describe, expect, it } from 'vitest'
import { UPLOAD_DIR } from '@/app/utils'
import mockFs from 'mock-fs'

describe('GET /api/run/results', () => {
const mockFileContent = 'header1,header2\nvalue1,value2'
const mockFileName = 'test.csv'

// Create a temporary test directory and file before each test
beforeEach(() => {
// Use mockFs to create the file in the mocked directory
mockFs({
[UPLOAD_DIR]: {
[mockFileName]: mockFileContent, // Mock the file directly
},
})
})

it('should return parsed CSV data when files exist', async () => {
const response = GET()
const data = await response.json()
expect(response.status).toBe(200)
expect(data.runs).toEqual({
[mockFileName]: [{ header1: 'value1', header2: 'value2' }],
})
})

it('should return empty runs when the directory does not exist', async () => {
// Simulate the directory not existing by removing it from mockFs
mockFs({
[UPLOAD_DIR]: {},
})

const response = GET()
const data = await response.json()

// Verify the response is an empty object
expect(response.status).toBe(200)
expect(data.runs).toEqual({})
})
})
28 changes: 12 additions & 16 deletions src/app/api/run/results/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,25 +8,21 @@ import { parse } from 'csv-parse/sync'
export const revalidate = 0

export function GET() {
try {
const runs: Record<string, any[]> = {}
const runs: Record<string, any[]> = {}

if (fs.existsSync(UPLOAD_DIR)) {
const files = fs.readdirSync(UPLOAD_DIR)
if (fs.existsSync(UPLOAD_DIR)) {
const files = fs.readdirSync(UPLOAD_DIR)

for (const file of files) {
const filePath = path.join(UPLOAD_DIR, file)
const fileContent = fs.readFileSync(filePath, 'utf-8')
for (const file of files) {
const filePath = path.join(UPLOAD_DIR, file)
const fileContent = fs.readFileSync(filePath, 'utf-8')

runs[file] = parse(fileContent, {
columns: true,
skip_empty_lines: true,
})
}
runs[file] = parse(fileContent, {
columns: true,
skip_empty_lines: true,
})
}

return NextResponse.json({ runs: runs })
} catch (error) {
return NextResponse.json({ error: 'Unable to read files' }, { status: 500 })
}

return NextResponse.json({ runs: runs })
}
Loading
Loading