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

Issue 193 #194

Closed
wants to merge 4 commits into from
Closed

Issue 193 #194

wants to merge 4 commits into from

Conversation

herpaderpaldent
Copy link

@herpaderpaldent herpaderpaldent commented Jan 29, 2024

Adding failing tests regarding #193

@Nielsvanpach
Copy link
Member

Can you run the tests locally and verify if the generated snapshots are correct?

@herpaderpaldent
Copy link
Author

Hi @Nielsvanpach

I don't understand how the snaps would help.

You see in the failed tests that the current implementation is incorrect:

I don't understand how the snaps would prove the missing of a holiday or the edge cases that are tested in the test?

@Nielsvanpach
Copy link
Member

You've added a bunch of tests, but the PR is missing snapshots. I'm closing this for now, but feel free to re-open the PR when it's not in Draft mode anymore.

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