Expand time travel

[lifeisafractal]

Overview

This PR brings wider support for time travel and time based callback registration in the test framework as covered in #18. This PR has some large changes to the internals of how the state of Hass objects are tracked and also proposes changes to the workflow consumers of the test framework will use. Currently, the PR is up to discuss these changes and get consensus before going much further with this work. There are a lot of code and architectural changes proposed in this PR, but they are just my stab at it and should be viewed as a starting point for discussion.

Proposed architecture changes:

The hass_fucntions fixture gets replaced with hass_mock fixture

• hass_mock test fixture is an instance of HassMock class which keeps track of the mocked Hass state and provides public interfaces to manipulate the simulated automation envrionment
• Add new new internal classes to help track separated internal state of Hass. For now this is just SchedulerMocks, but could be extended to include things like StateMocks, EventMocks, etc.
• Maintain backwards compatibility with existing code by constructing the hass_fucntions dict internally. I propose that this interface be considered deprecated in favor of public methods in the HassMock class as this PR does with time_travel

Create the concept of pre and post initialization of automations. Although not fully enforced in this PR, this is the proposed new patterns that guided the new implementation of time_travel:

• Set initial stage with the given_that fixture. Time, state, automation parameters, etc.
• Automations are initialized. (after this given_that cannot be used)
  • Right now this is either done by @automation_fixture or by the user calling initialize manually.
  • In the future it might make sense to group this into a fixture to make tests more clear to read and write. For example, you do all your setup and then call given_that.setup_complete(), given_that.automations_initialized(), or something similar.
    • Another option would be to lazily initialize automations on the first call to something that needs that and block future calls to given_that. This would be less obvious, but also less verbose for the user.

      def test_thing(automation, given_that, time_travel):
      given_that.time_is(datetime.time(15,00))
      # implicit initialization of automations with the call to `time_travel`.
      # No more calls to `given_that` allowed after this.
      time_travel.fast_forward(15).minutes()

• Stimulate and observe (can be interleaved)
  • Stimulate the simulation (time_travel, state changes, events, etc)
  • Observer the simulation (assert_that) The above process uses the test fixtures that already exist such as given_that, time_travel, and assert_that. The idea would be to keep these and add one as needed to keep test code highly readable.

To deal with the above pre/post initialization, it is useful to be able to create @automation_fixtures that do not initialize. An initialize key word argument was added to @automation_fixture to allow skipping the call to the auotmations' initialize function. This allows the user to do further setup with given_that and then manually call initialize.

Also, there is protection code added to enforce these orderings. For instance, if you try to time travel with registered callback it will raise an Exception. There are a few reasons for doing it this way:

• It guides the user to consume the test library correctly
• Exceptions give clear feedback on the failure so the test writer can correct. (e.g. "You can not set_time() after automations have been initialized.")

External facing changes

Although the aim was to limit externally facing changes to users of this code, some changes were necessary that will impact consumers of this library.

Due to the more detailed simulation of time, there are some new ordering dependencies that need to be honored by the consumer.

• Start time can only be set prior to initializing automations
  • Once any callbacks have been registered, jumping around in time would cause inconsistencies in the simulated state.

The effects of these changes can be seen in how the bathrrom integration example tests don't really work anymore and some tests needed to be skipped. This is due to these tests being written before there was concept of a pre/post initialization stage. I'm looking for input on how best to cover this, but I think it might just be refactoring this test code to fit in these paradigms.

Questions

This PR adds test coverage through the various user facing test fixtures (assert_that, time_travel, etc). It also adds a few tests for individual internal classes, but there felt to be some overlap in this area. Any thoughts on what best practice is here?

Remaining work after design decisions are ironed out:

• [x] Merge in master to get the Hass.name change
• [ ] Update or re-write the bathroom integration tests example to fit new paradigms
• [ ] Finish updating documentation

[HelloThisIsFlo]

Intro

I am impressed by the care you put in the code you write, and am really happy to have someone like you participate to this repo.

I have read the code quickly but didn't get into the details. As I was reading I quickly realized: This is a very professional PR, regarding the code as well as the description. So I figured, let's save both of us a lot of time and collaborate as professionals. I do not need to oversee all the nitty gritty details of your code: I trust your work and for now I'd rather discuss on the overview. At the very end I'll review everything in details just to help you for things like typo and such, or things difficult to see with only one pair of eyes ;) (only if you wish so)

For that purpose, in my opinion there is not better way than to meet, via video or simple screen sharing. Given your enthusiasm I'm sure you could present your work in a much more efficient manner than if I tried to understand it all on my own. Not that I couldn't, your code is very readable, I just think it would accelerate things for the both of us.

I'll add a doodle link with my availabilities below.

Comments

That being said, I did read your explanatory text and I have a couple of comments.

Backwards compatiblity: hass_function

Constructing it internally

Great idea!

Declare deprecated and move away from it

Yes definitely, I am 100% in line with that. I really like your approach of maintaining compatibility but slowly fading it away. When that particular version with all your changes get released, we could print warnings to indicate the deprecation, and couple of versions later throw explicit exception explaning how to move away from hass_functions and use HassMock instead.

My two previous comments assume that HassMock works as expected and is not in an experimental state, but I think that's obvious for the both of us :)

Pre-post initialization

General idea

This is quite a breaking change, but if it brings value I'm totally in to go down that road. Of course we'll think of a release strategy together so that people who have tests like the bathroom example do not end up frustrated. Conceptually I think it makes a lot of sense, and if it helps keep everything coherent, yep 100% behind the idea.

Manual / Delayed initialization

I agree, that there must be some mechanism to allow delayed initialization. Otherwise, it would mean all the setup for any test would have to go in the @automation_fixture body, which wouldn't work if we had more than one test per automation, so in like 99% of cases.

As to how to implement it in practice, I think given_that.setup_complete() or something of the sort is a good solution. Also, at first I was rather opposed to the other option of initializing in the background. I felt it was too "magic" and a nightmare for the user to debug. But then I thought, if we implement this properly, if it's clearly documented, and if whenever the user does the unexpected we throw an exception with a very detailled explanation of why this happens, linking to the documentation, then .... then I think it might actually be an even better option than the explicit initialization.

But well, I think these are the sort of things that would be easier to discuss in person :)

initialize keyword in @automation_fixture

Yep sounds good, and following a gradual deprecation process we could even make the default value for this parameter: false. So that the new Pre-Post intialization would be the standard way of thinking about the test framework

Protection code

. . . do I need to comment? 😊 This is exactly what I was suggesting a couple of lines above. So yes, I love the "fail fast" / "let's teach the user in real time" approach. 👍

External facing changes

Let's discuss this in person and figure out a strategy that would make sense in terms of how to release the breaking change without annoying too much the couple of users who decided to check out this project in its early stage :).

Question about test strategy

We can get into that after you give me your overview of the code and discuss about it. I am not pretending to have all the answers, but I think we can come up with something interesting with a discussion on the topic :)

Outro

After all I decided to not go with a doodle, let's just talk and decide when it would work best ;)

Could you join the Software Craft slack?
=> http://slack.softwarecrafters.org/
It's a very welcoming community, I'm sure you'll feel right at home. And more importantly you'll be able to DM me there, so we can schedule a meeting. Also feel free to reach me if you have a quick question or need input on something ;)

Overall, I know I say it a lot, but you deserve it: Thanks a lot for your work! It's amazing 😃 Looking forward to collaborate on that

[lifeisafractal]

Hi Florian, I just joined the Software Crafters Slack and will DM you on there in a bit. I'll also digest some of this great feedback and see if I can summarize any of my thoughts before we talk. Also, thanks for the praise; it's a lot of fun working on this project and you're an absolute pleasure to work with :).

[HelloThisIsFlo]

Hey 🙂

I just wanted to let you know that I've contacted you via DM on the Software Crafter Slack. If you're not available, there is absolutely no rush. But I figured since you didn't answer for 2 weeks maybe you never saw my messages 😅 And if it's a time problem, I repeat, there is no rush 😊

[lifeisafractal]

Whoops, thanks for dropping a message here. I thought Slack would give me email notifications on DMs. I'll check into that in a bit and get back to you there.

[HelloThisIsFlo]

Haha all good. I'm glad you eventually got my message ;) And again, there's no rush. Whatever timing we manage to make work, will be the best timing :)

[HelloThisIsFlo]

Hey, Any news on this? I'd like to re-state: There is absolutely no rush 😊 I'm just pinging 🙂

[lifeisafractal]

No problem, thanks for the check in! Life had a way of getting away from me and to be honest the async communication worked better for me. I've got some time over the holiday that I can work on this. I'll need to get back up to speed on my changes and then we can sync up.

I'm really looking forward to landing some of these changes, I recently switched all my lighting over to smart switches and would like to start getting some more elaborate automations going with testing form day 1!

[HelloThisIsFlo]

All good 👍

The idea was to continue working async, but a call is important to discuss certain aspects that would take ages to talk about over text. Plus I'd like you to introduce me to your changes and their logic instead of figuring it out on my own 🙂 Also, the goal of the call is not to micromanage you, no worries. On the contrary, the goal would be to set ground rules to allow you to work more independently on my project, and maybe give you some permissions to the repo 🙂👍

Anyways, let's get in touch and synchronize via slack for a meeting time and date. But again, no rush, could be 2 months from now I'm totally ok with that 🙂👍

Happy holidays 😊

[lifeisafractal]

That all sounds great, I'll drop you a message on slack.

[lifeisafractal]

Just an FYI, I merged in upstream master to my branch and got all tests passing again. This is ready for review/discussion.

[lifeisafractal]

After talking about this PR, I decided it's a better path to break this down in a series of smaller PRs. This will help greatly with review of these PRs since this one got to be a bit of a monster. First in the line of PRs is #27 .

I'll still be using this branch to guide the coming series of PRs to ultimately get to the new time travel framework.