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

Fix tests for windows, which appears to have a lower resolution time #17

Merged
merged 2 commits into from
Dec 4, 2024

Conversation

sfider
Copy link
Contributor

@sfider sfider commented Nov 25, 2024

Tests have around 19 fails when run on windows. Mostly OSError: [Errno 22] Invalid argument on 4000 timestamps (too low resolution time).

test_push also had issues with recorded times comparing as equal (again, low resolution time). But I also noticed it used arrow.now() while tested code uses arrow.utcnow(), so I fixed this as well.

@teutat3s teutat3s requested review from teutat3s and b12f November 27, 2024 16:04
Copy link
Member

@teutat3s teutat3s left a comment

Choose a reason for hiding this comment

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

Thank you for fixing this. Could you fix the linting issues? Then this would look good to go from my side.

@sfider
Copy link
Contributor Author

sfider commented Nov 27, 2024

Thank you for fixing this. Could you fix the linting issues? Then this would look good to go from my side.

Sorry, I was going to, but I'm really swamped with work. I would have to figure out how to run lint test locally not to push something that would fail again XD

I'll try to get to it on the weekend maybe.

@teutat3s
Copy link
Member

No worries and no rush. This is the command we run in the CI:

flake8 --show-source watson/ tests/ scripts/

@b12f b12f merged commit 9e56857 into pub-solar:main Dec 4, 2024
7 checks passed
@b12f
Copy link
Member

b12f commented Dec 4, 2024

Thanks @sfider!

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.

3 participants