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

[testing] ContextMock (#1976) #2038

Merged
merged 38 commits into from Dec 6, 2023

Conversation

dzikoysk
Copy link
Member

@dzikoysk dzikoysk commented Nov 4, 2023

Based on (& blocked by) branch from 1st PR:

I don't quite like xxx-utils libraries, because it's usually a garbage can for random things and no one knows what's actually there.

I took a different approach and made javalin-utils a parent pom, where javalin-context-mock is its first child module. Thanks to that, we can still encapsulate utility features within the -utils scope in the repository, but it's effectively still granulated & independent functionality.


I think it might be even a good idea to pull javalin-utils/javalin-testtools here, maybe with a more precise name like javalin-okhttp-testtool/javalin-okhttp-tester (?)

@dzikoysk dzikoysk changed the title GH-1976 ContextMock [testing] ContextMock (#1976) Nov 4, 2023
@dzikoysk dzikoysk marked this pull request as draft November 4, 2023 00:39
@dzikoysk
Copy link
Member Author

dzikoysk commented Nov 4, 2023

Classic Maven incident and its inability to get the root directory for multimodule project 😔 I'll play with it tomorrow.

@dzikoysk dzikoysk marked this pull request as ready for review November 4, 2023 13:16
# Conflicts:
#	javalin/src/main/java/io/javalin/http/servlet/JavalinServlet.kt
#	javalin/src/main/java/io/javalin/http/servlet/JavalinServletContext.kt
@tipsy
Copy link
Member

tipsy commented Nov 16, 2023

I'll try to have a proper look at this in the weekend!

# Conflicts:
#	javalin/src/main/java/io/javalin/plugin/bundled/DevLoggerPlugin.kt
Copy link
Member

@tipsy tipsy left a comment

Choose a reason for hiding this comment

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

This looks really great @dzikoysk 🙌

@dzikoysk
Copy link
Member Author

dzikoysk commented Dec 5, 2023

Probably done, I mean... it's not a real req/res impl so there are definitely some differences, but I guess we can fix this on user request. The foundation seems to work quite fine :)

@tipsy tipsy merged commit 967c4df into javalin:master Dec 6, 2023
9 checks passed
@tipsy
Copy link
Member

tipsy commented Dec 6, 2023

Thank you for all your effort @dzikoysk, let's make sure we promote this to get some user feedback 🙌

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.

None yet

2 participants