test(replay): fix full session replay#116
Conversation
The issue is a bit subtle: the event notification about the session changing from nil to an id gets wrapped in vim.schedule but that means it gets delivered after we've loaded the fulls session, which causes us to clear the whole session right after we just loaded it, leaving the output blank. It's possible that we might want to not wrap state notifications in vim.schedule and expect listeners to handle cases where they can't be in a fast context. The primary benefit of that would be guaranteed ordering of events. For example, in this case, the event that the session was set would've been processed before we loaded the session so the issue wouldn't exist.
|
Actually, the more I think about this the more I think it's probably not an issue in practice. The replay harness pattern of setting the session and then manually loading the session directly has the order and effect backwards. Normally, it's setting |
You are right about this. It makes more sense this way |
|
Sounds good. This PR should still be merged as it just fixes the full session replay. |
2 participants
The issue is a bit subtle: the event notification about the session changing from nil to an id gets wrapped in vim.schedule but that means it gets delivered after we've loaded the full session, which causes us to clear the whole session right after we just loaded it, leaving the output blank.
It's possible that we might want to not wrap state notifications in vim.schedule and expect listeners to handle cases where they can't be in a fast context. The primary benefit of that would be guaranteed ordering of events.
For example, in this case, the event that the session was set would've been processed before we loaded the session so the issue wouldn't exist.
The issue is only coming up now because the replay script used to unsubscribe to
state.active_sessionchanges but i didn't keep that because it felt like a hack.For now, I think we can just watch out for any issues in actual usage.