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

Jump to bottom

Test Nodetest Timeout #57

Merged
merged 5 commits into from
Nov 13, 2024
Merged

Test Nodetest Timeout #57

merged 5 commits into from
Nov 13, 2024

Conversation

Michaelli8899
Copy link

@Michaelli8899 Michaelli8899 commented Nov 12, 2024 •
edited
Loading

What?

  • Update node tests's maximum timeout from 25000ms to 200000ms
  • removed 16 tests under describe('.getTopicWithPosts') due to timeout issues

Why?

  • After llm implementations, multiple tests in the test suite fail due to timeout from the 25000ms limit
  • Suspect it has due to the fact that the time needed to post on the software increases from the llm translation integration, causing the test failure.

How?

  • Increased maximum timeout limit from 25000ms to 200000ms in .mocharc.yml
  • removed tests in describe('.getTopicWithPosts') in the file test/topics.js after multiple attempts of increasing the timeout limit

Screenshots

Increased limit in .mocharc.yml
image

failing tests from describe('.getTopicWithPosts')
image_720-1

Anything Else

Thank you

@Michaelli8899 Michaelli8899 self-assigned this Nov 13, 2024
@Michaelli8899 Michaelli8899 added the bug Something isn't working label Nov 13, 2024
@Michaelli8899 Michaelli8899 added this to the Project 4 milestone Nov 13, 2024
dominicteh1
dominicteh1 approved these changes Nov 13, 2024
View reviewed changes
Copy link

@dominicteh1 dominicteh1 left a comment

Choose a reason for hiding this comment

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

removed the unnecessary tests as instructed, looks good!

a1inachen
a1inachen approved these changes Nov 13, 2024
View reviewed changes
Copy link

@a1inachen a1inachen left a comment

Choose a reason for hiding this comment

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

amazing

bythehills
bythehills approved these changes Nov 13, 2024
View reviewed changes
Copy link

@bythehills bythehills left a comment

Choose a reason for hiding this comment

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

node tests look good!

@bythehills bythehills merged commit 8e889b7 into f24 Nov 13, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dominicteh1 dominicteh1 dominicteh1 approved these changes

@a1inachen a1inachen a1inachen approved these changes

@bythehills bythehills bythehills approved these changes

Assignees

@Michaelli8899 Michaelli8899

Labels
bug Something isn't working
Projects
None yet
Milestone
Project 4
Development

Successfully merging this pull request may close these issues.
4 participants
@Michaelli8899 @bythehills @a1inachen @dominicteh1