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

test: add test for trip existence #901

Merged
merged 3 commits into from
Nov 3, 2024
Merged

test: add test for trip existence #901

merged 3 commits into from
Nov 3, 2024

Conversation

YaelChen
Copy link
Collaborator

Description

My first test, created by codegen,
Choosing params under "קיום נסיעות" and organize by date/hour.

@NoamGaash
Copy link
Member

Thanks for the contribution! Truly appreciate it.
Please run npm run lint:fix as described here

@NoamGaash NoamGaash changed the title YaelChen add test for trip existence test: add test for trip existence Oct 30, 2024
Copy link
Member

@NoamGaash NoamGaash left a comment

Choose a reason for hiding this comment

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

lgtm, can be merged once the commit statuses are fine

@YaelChen
Copy link
Collaborator Author

Thanks @NoamGaash, forgot about the lint. only installed it now.
do I need to run it and push again without the useless "expect" (that I first thought i'll leave for future use)?

@NoamGaash
Copy link
Member

Exactly, thanks!
once the lint issues are solved, the test statuses should be green

@YaelChen
Copy link
Collaborator Author

YaelChen commented Nov 3, 2024

It still fails,
something about a render component I don't know?

  22 |   let renderedComponent: RenderResult
  23 |   beforeEach(() => {
> 24 |     renderedComponent = render(
     |                               ^
  25 |       <ArrivalByTimeChart data={testBusData} operatorId={testBusData[0].id} />,
  26 |     )
  27 |   })

it does mention I need to ask the APPLITOOLS_API_KEY, as it is not defined. don't know if it has something to do with it.

@NoamGaash
Copy link
Member

Hi Yael, it's not you fault and I apology for the bad experience, I have probably misconfigured something with the visual tests - the error Test timeout of 180000ms exceeded while running "afterEach" hook indicate that we have some performance problems, it's not really test failures.
I pushed a commit (121e6d5) to ignore this error and remove the timeout, and I'll try again.

regarding the API key - it's just a warning - the visual tests are skipped by default when your API key is not configured on your machine. If you want to execute the visual tests I'll send you the API key but it's really not that important, I hope that the fix I pushed to main will solve this error

@NoamGaash
Copy link
Member

Great job! Thanks you 👏
Feel free to merge

@YaelChen YaelChen merged commit e926707 into main Nov 3, 2024
15 checks passed
@YaelChen YaelChen deleted the newTest_tripExistence branch November 3, 2024 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants