-
Notifications
You must be signed in to change notification settings - Fork 757
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(ci): unittest failed #2908
fix(ci): unittest failed #2908
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2908 +/- ##
==========================================
+ Coverage 70.88% 70.93% +0.05%
==========================================
Files 103 103
Lines 9335 9334 -1
==========================================
+ Hits 6617 6621 +4
+ Misses 2718 2713 -5
|
time.sleep(1) | ||
DummyItem.create("test:otherprefix") | ||
time.sleep(1) | ||
DummyItem.create(Tag("test", "version2")) | ||
time.sleep(1) | ||
DummyItem.create("test:version3", creation_time=oldtime) | ||
time.sleep(1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No other better options? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering why this issue didn't surface earlier
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to be pertinent to Windows so it may have to do with the OS clock API?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it may be fine to mock creation_time
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just open this PR on behalf of sauyon's suggestion. The current CI is broken, which false-positive all current PRs. But it seems that there's more issue in the unittest about tag version.
Let me hand it over to sauyon
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ssheng BTW to me mock creation_time will also decrease the coverage literally. Deciding the creation_time according to real-world time is an important part of the logic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I remember now we tried to fix this by overwriting latest if the timestamp is the same.
I'm really not sure what's happening here...
No description provided.