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

Improve Test Coverage #74

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

varshith257
Copy link

@varshith257 varshith257 commented Jan 24, 2025

Purpose

This PR aims to increase the test coverage of the StarbaseDB project to 75%+ using Vitest. The improvements ensure meaningful tests that not only improve line coverage but also help detect potential issues by testing different scenarios and edge cases and fixed few of them too

Tasks

  • Added unit tests

  • Fixed found issues too

  • [ ]

Verify

  • Run the test suite using:
pnpm vitest run --coverage
  • Confirm that all tests pass and coverage has increased

Before

After

/claim #71

@varshith257
Copy link
Author

@Brayden PTAL. I will test each function cleanly, push the code from on and have fixed few issues found too

@varshith257
Copy link
Author

@Brayden Test coverage is good so far and few issues have been found in the DELETE operation so I have commented. I will take a close look fix it and improve some more files' test coverage to reach ~85-90

@varshith257
Copy link
Author

@Brayden It's weekend and I am out . I will push final remaining changes by tomorrow

@Brayden
Copy link
Member

Brayden commented Jan 26, 2025

@varshith257 No worries. I was just updating your PR so it didn't have merge conflicts. I introduced a couple test changes so a report could be displayed on new PR's. I ran the test coverage here and figured I'd share:

image

Also do you recall what error you were getting for DELETE operations? Curious to what bug was uncovered!

@varshith257
Copy link
Author

@Brayden I can't remember what exactly found as I kept in a notes in my desktop but what i remember is it throwing bad request instead executing delete operation

For example check this


        // it('should handle DELETE requests successfully', async () => { 
         //     vi.mocked(executeQuery).mockImplementation(async ({ sql }) => { 
         //         console.log('Mock executeQuery called with:', sql) 
  
         //         if (sql.includes('PRAGMA table_info(users)')) { 
         //             return [{ name: 'id', pk: 1 }] 
         //         } 
  
         //         return [] 
         //     }) 
         //     vi.mocked(executeTransaction).mockResolvedValue([]) 
  
         //     const request = new Request('http://localhost/rest/users/1', { 
         //         method: 'DELETE', 
         //     }) 
         //     const response = await liteRest.handleRequest(request) 
  
         //     console.log('DELETE Test Response:', response) 
  
         //     expect(response).toBeInstanceOf(Response) 
         //     expect(response.status).toBe(200) 
  
         //     const jsonResponse = (await response.json()) as { 
         //         result: { message: string } 
         //     } 
         //     console.log('Parsed JSON Response:', jsonResponse) 
  
         //     expect(jsonResponse.result).toEqual({ 
         //         message: 'Resource deleted successfully', 
         //     }) 
         // })

and this too

        // it('should return 400 for DELETE without ID', async () => { 
         //     const request = new Request('http://localhost/rest/users', { 
         //         method: 'DELETE', 
         //     }) 
         //     const response = await liteRest.handleRequest(request) 
  
         //     expect(response).toBeInstanceOf(Response) 
         //     expect(response.status).toBe(400) 
  
         //     const jsonResponse = (await response.json()) as { error: string } 
         //     expect(jsonResponse.error).toBe( 
         //         "Missing primary key value for 'id'" 
         //     ) 
         // }) 
 

It throwing 500 instead 400 above and delete query succes. I need to go another round of their implementation to validate whether I am wrongly testing the method or need a fix there. To capitalise i have commented and just console logged their results but here mock data receiving perfectly to the DELTE but when executed final result is getting undefined

@Brayden
Copy link
Member

Brayden commented Jan 30, 2025

@varshith257 Wanted to check in and see if you had more ready to push up or not! :)

@varshith257
Copy link
Author

@Brayden Sorry for the inconvenience. Actually I had on holiday from last 3 days so didn't push changes here. I am back and on the work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants