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

Feature: session handling #44

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

harijoe
Copy link

@harijoe harijoe commented Aug 22, 2017

No description provided.

@harijoe harijoe force-pushed the feature/session-handling branch from c3f96c8 to 8ab5b45 Compare August 22, 2017 15:35
@alexjfno1
Copy link
Collaborator

@harijoe I've had a look at your PR and it sounds like a cool idea that you could have sessions with different mocks in them.

I'm currently working on a version 3 of Simulado ( Which is basically a total rewrite ). I'd be keen to take what you've done and port it over to v3.

Just one question: How does the session Id get managed? Is that the clients job to do that or is that the job of simulado to manage that? Personally I think it would be the responsibility of the client to manage it's own session and pass that to simulado.

@ldabiralai
Copy link
Owner

Thanks @harijoe, sorry this has taken a while to be looked into.

Just one question/thought. Ideally defaultMocks should be called after each test to ensure tests don't overlap with each other and I'm not sure this works when doing this.

Let's imagine that we have two tests simultaneously running using different sessions, as implemented here. The first test finishes and calls defaultMocks, causing the mocks to be reset. This causes the still running test to have it's mocks reset and therefore fail.

I'm not totally sure how we can get around this issue without adding further complexity around having defaultMocks tied to a session too, which I'm not too keen on. I wonder if we could solve this some other way?

My initial thought regarding this feature was to spin up a new simulado in the before hook of your test, test against it and then spin it down after? Obviously this moves a bit more complexity to the more advanced consumer, so I'm more than willing to consider alternatives!

@bdo
Copy link
Collaborator

bdo commented Oct 18, 2017

@alexjfno1:

How does the session Id get managed?

I talked with @harijoe about this and the general idea was to get the client to generate the session id themselves.

@bdo
Copy link
Collaborator

bdo commented Oct 18, 2017

@ldabiralai

Let's imagine that we have two tests simultaneously running using different sessions, as implemented here. The first test finishes and calls defaultMocks, causing the mocks to be reset. This causes the still running test to have it's mocks reset and therefore fail.

The solution is simple: you would call defaultMocks with the session id.

@bdo
Copy link
Collaborator

bdo commented Oct 18, 2017

@harijoe the PR looks messed up and needs rebasing to remove the erroneous commits (we should not be able to see @vlebadezet and @ldabiralai commits here) 😉

@alexjfno1
Copy link
Collaborator

@bdo Ok, I think the client handling the sessionId is the best way as well. Unfortunately this PR was done against the v2 version of Simulado so it might be better to take a fresh version of v3 and copy code across as the whole structure has changed.

So if the client handles the sessionId they will have to send it on every request to Simulado right? Possibly through a header in the request like X-Simulado-SessionID?

@ldabiralai
Copy link
Owner

ldabiralai commented Oct 19, 2017

The solution is simple: you would call defaultMocks with the session id.

Ahh yes.. sorry, I was just putting thoughts out there while on the train so didn't see the PR accounted for this already.. 😄

I'll have a look at getting this PR (or a similar version of it) into both v2 & v3 tonight.

Possibly through a header in the request like X-Simulado-SessionID?

This should work, I'm not sure how easy it would be for the client to handle injecting that into each request though, but I guess it's the only option aside from splitting tests across multiple machines.

@ldabiralai
Copy link
Owner

ldabiralai commented Oct 19, 2017

This should work, I'm not sure how easy it would be for the client to handle injecting that into each request though, but I guess it's the only option aside from splitting tests across multiple machines.

An alternative to this is to respond to a cookie, as it's a fair bit easier to set these during browser tests (as you wouldn't need to modify the actual code to inject a header), this is how this PR is implemented.

The downside of that is the cross domain issues, meaning that simulado would have to run on the same hostname as the app making requests, would this be practical?

@bdo
Copy link
Collaborator

bdo commented Oct 19, 2017

The downside of that is the cross domain issues, meaning that simulado would have to run on the same hostname as the app making requests, would this be practical?

I think we can live with a cookie to start with. We may want to open an issue later to address this concern. The X-Simulado-SessionID header is definitely a good solution for this. But like you said the downside is we'd have to change the production code.

If you want to go for both cookie or header in the initial PR I'm OK with that as well 👍

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.

4 participants