Fix tests for isAuth.spec.ts and src/server.ts

[palisadoes]

Describe the bug
We have been getting periodic errors where our GitHub action testing fails. Re-running the tests often causes the error to go away.

1. This is an example of one such failed test run.
2. Click through and you will see the screen shot below.

Other

1. This has been fixed with fixed express static files path in server.ts #1007 however we need to ensure that this test correctly verifies the existence of the required resources to reduce any future failure scenarios.
2. That PR made modifications to src/server.ts This may mean that the tests for that file need to be fixed too.

To Reproduce

1. Submit a PR
2. tests/middleware/isAuth.spec.ts will occasionally fail.

Expected behavior
Tests pass reliably and consitently

Actual behavior
See above

Screenshots

Additional details
N/A

[palisadoes]

You already have 2 issues assigned. Please wait until your other issues are closed via a PR merge

[KrutikaBhatt]

Hey, can I work on this issue?

[palisadoes]

Please look at duplicate issue #1025 for any further details

[KrutikaBhatt]

@palisadoes, yes I will take a look. Thank you so much

[ayushbhaimehta]

Hey if no one is working on this then I would love to work on this please tell from where should I begin.

[palisadoes]

@ayushbhaimehta This issue was assigned just yesterday. Please take a look at other unassigned issues or consider creating one using these guidelines PalisadoesFoundation/talawa#359

[kb-0311]

@palisadoes If I am not wrong was this not fixed by -> 93e01bd this commit of this PR ?

I was getting the same error on that PR and it was because of uploadImage.ts so I put all fs mutations in try catch block which seemed to get rid of the error then.

Is the error still there?

[palisadoes]

1. Yes it's still happening.
2. There is no /images directory in the repo.
3. There is /images. Also we should not be uploading images into the code tree.

[kb-0311]

1. I highly doubt the error is from isAuthSpec.ts as it is the last test executed perfectly before the ENONET error. Even in my PR where the tests were failing, the main problem was not that file.
2. I think the main problem is in the uploadImage.ts file since I wrote the code to create the /images directory there for testing purposes. And for some reason it is inconsistent still if we have records of it still happening.
3. @xoldyckk and I coordinated the directory structure as well after which the tests seem to pass. So I also doubt it is because of src/server.ts since the actual ENONET error path is -> '/home/runner/work/talawa-api/talawa-api/images/image.png' the image.png is a mock file created in and used only for testing purposes in uploadImage.ts and deleted afterwards after the file has been executed.

This is an example of one such failed test run.

After this and #1007 were merged I am curious whether the error was thrown in some other PR also (if yes could you pls link the action run), or locally instances or both , we need to know for sure to see what is the exact problem here.

@xoldyckk DO you have any input here as well?

[palisadoes]

An example of the failed action is linked in the issue's comment

[kb-0311]

@palisadoes Yeah I know, I was the author of that PR, and what I am saying is that in the commit after the failed test ie. the ENONET error was thrown here you told me to look into it, ->#998 (comment) , and I did after the ENONET error was cleared I was sure I had fixed it so in the end all the tests had passed.

Here is the original PR -> #998 as you can see the tests are passed
this PR was a complementary PR to #1007 ,
#1007 (comment)

That's why I was asking for instances of the tests throwing this error after that PR was merged.

[palisadoes]

I'll close this and if it returns I'll reopen it

[kb-0311]

@palisadoes sounds good to me.

@KrutikaBhatt Sorry about this one, I hope you did not beat yourself up over this issue. You can look for other issues to get assigned and contribute to.

[palisadoes]

@kb-0311 It's happened again. This needs to be resolved please see this run:
https://github.com/PalisadoesFoundation/talawa-api/actions/runs/4137414342/jobs/7152552533#step:7:168

[palisadoes]

[kb-0311]

@palisadoes The reason it failed here is that writeFIle , createReadStream are asynchronous methods and so for that, to correctly execute in vitest , it needs all the other tests to pass, as weird as it may sound otherwise vitest cannot track it hence , unhandled.

Source - vitest-dev/vitest#1692 (comment)

In that test run, there were other tests that were failing and hence the file handling error was thrown.

I am gonna ask in this thread about the best possible approach to tackle this.

I wonder if we can wrap the entire test suite in try-catch for this

[kb-0311]

Ok nevermind, got a possible fix in the comment itself, but I needed to add an extra parameter in the vite. config.ts file and another file for this to work

[xoldd]

@kb-0311 tasksByEvent test probably failed because you changed the logic for getSort function and it no longer returns a sort object expected by tests when args.orderBy === undefined.

For example previously args.orderBy === undefined would've resulted in getSort returning { deadline: -1 } as the sort object. But now it returns { }. Since you didn't refactor the tests relying on that logic that's probably why this error occured.

You have to remove all the test blocks that rely on this logic like this one:-

returns list of all tasks with task.creator === args.id and sorted by descending order of task.deadline if args.orderBy === undefined

[kb-0311]

@xoldyckk Oh yeah, that is one test suite I missed while removing that orderBy=== undefined test. Which will be fixed in that same PR. Thanks for bringing that up.

I think the main problem was that even though a separate test suite failed the unhandled error was thrown for the image file generated in vitest for uploadImage.ts.

#1010 (comment)