Skip to content

Add smoke test with Spring Session and Hazelcast #28173

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

Closed
wants to merge 3 commits into from

Conversation

kandulsh
Copy link

@kandulsh kandulsh commented Oct 1, 2021

Added smoke test with spring session and Hazelcast

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 1, 2021
@snicoll snicoll changed the title Issue 27793: Add smoke test with Spring Session and Hazelcast Add smoke test with Spring Session and Hazelcast Oct 2, 2021
@mbhave mbhave added type: task A general task and removed status: waiting-for-triage An issue we've not yet triaged labels Oct 5, 2021
@mbhave mbhave added this to the 2.6.x milestone Oct 5, 2021
mbhave pushed a commit that referenced this pull request Oct 6, 2021
@mbhave mbhave closed this in 1ff900c Oct 6, 2021
@mbhave
Copy link
Contributor

mbhave commented Oct 6, 2021

@kandulsh Thanks for the pull request! This has now been merged into main along with this polish commit which added some changes to assert on the number of sessions.

@mbhave mbhave modified the milestones: 2.6.x, 2.6.0-RC1 Oct 6, 2021
@vpavic
Copy link
Contributor

vpavic commented Oct 8, 2021

IMO the tests are more complicated then necessary as they are testing the sessions endpoint itself (and that's already tested elsewhere). It would be enough to assert the presence of SessionsEndpoint bean (and maybe expected session repository bean) in the application context - see the example from my own Spring Session samples.

@wilkinsona
Copy link
Member

Thanks for taking a look, Vedran. I prefer the test as currently written. At the level of a smoke test, I think it's better to check that a particular feature is working as a user would expect, rather than checking for the presence of some underlying components that enable that feature.

@vpavic
Copy link
Contributor

vpavic commented Oct 8, 2021

Fair enough - I didn't consider the user perspective to still be of primary interest here since these aren't considered samples so I focused on the actual touching points of Spring Boot's integration for Spring Session.

@mbhave
Copy link
Contributor

mbhave commented Oct 8, 2021

What @wilkinsona said is my preference for smoke tests as well. Even if they aren't meant to be consumed as samples, the tests should test what the high-level feature is to make sure nothing in between is broken.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: task A general task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants