Skip to content

Conversation

adamchainz
Copy link
Contributor

It's faster :)

In this case, about 0.1 seconds are saved... there aren't that many tests using it.

Thanks for contribution

Please, make sure you address all the checklists (for details on how see
development documentation)!

  • ran the linter to address style issues (tox -e fix_lint)
  • wrote descriptive pull request text
  • ensured there are test(s) validating the fix
  • n/a added news fragment in docs/changelog folder
  • n/a updated/extended the documentation

@adamchainz adamchainz requested a review from gaborbernat as a code owner April 28, 2022 10:20
@jugmac00
Copy link
Member

Thanks, Adam!

@gaborbernat There was some discussion on Twitter, especially @pganssle had some concerns about time-machine.
https://twitter.com/AdamChainz/status/1513446705567571969

I think this needs a closer look, though a win of 0.1 s sounds pretty tempting :-)

@adamchainz
Copy link
Contributor Author

I don't think those concerns affect the tests here, which I think are freezing time purely to guarantee that all operations are seen as taking 0.0s for consistent output :)

Copy link
Member

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

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

Copy link
Member

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

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

Can you please a changelog for this otherwise looks alright

@adamchainz
Copy link
Contributor Author

Can you please a changelog for this otherwise looks alright

There aren't any other files in docs/changelog on the rewrite branch - are you sure?

Also I tend to avoid adding release notes for internal-only changes. Users don't need to read about how tox's own test suite has changed.

@gaborbernat gaborbernat merged commit e5ab3e6 into tox-dev:rewrite May 11, 2022
@adamchainz adamchainz deleted the time_machine branch May 11, 2022 15:21
@jayaddison
Copy link
Contributor

Related to this: there's some use of time.monotonic throughout the tox codebase, which isn't supported by time-machine -- I think it might explain some test flakiness recently (particularly on Windows 2022, for whatever reason)

@hroncok
Copy link
Contributor

hroncok commented Jan 9, 2023

We don't have time-machine packaged in Fedora, so we "simply" tried removing all the decorators and the tests passed. What is the purpose of this dependency?

@gaborbernat
Copy link
Member

I think the spinner tests elapsed field here

@time_machine.travel("2012-01-14", tick=False)
might change depending on how fast machine you're running

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants