feat: add framework-agnostic mock context for testing #351

olisaagbafor · 2025-01-13T18:03:30Z

Add MockContext for testing controllers

Adds a framework-agnostic MockContext implementation that allows users to easily test their Fuego controllers without setting up a full HTTP server.

Changes

Add MockContext implementation with all required interface methods
Add comprehensive testing documentation
Add example tests showing real-world usage

Closes #349

Implements a MockContext[B] type that allows testing controllers without framework dependencies. Includes comprehensive tests and documentation.

README.md

mock_context_test.go

EwenQuim · 2025-01-13T18:36:38Z

mock_context.go

+func (m *MockContext[B]) SetBody(body B) {
+	m.body = body
+}


Why not using a setter and not an exported attribute name?

I use a setter method instead of an exported field to maintain encapsulation and consistency with real implementations. This allows to add validation or logging in the future without breaking user code and ensures tests reflect how the code will behave in production.

Add MockContext[B] to help users test their controllers without framework dependencies. Includes realistic examples and documentation. - Move testing docs from README.md to TESTING.md - Add mock context with getters/setters - Add example user controller tests - Use fuego_test package for end-user perspective Closes go-fuego#349

Replace basic getter/setter tests with a real-world example: - Add UserController with Create and GetByID endpoints - Add UserService with mock implementation - Show proper separation of concerns - Demonstrate practical testing patterns - Move testing docs from README.md to TESTING.md This provides users with a clear example of how to test their controllers using the mock context in a real-world scenario. Closes go-fuego#349

TESTING.md

EwenQuim · 2025-01-13T23:03:09Z

Honestly I'm a bit disappointed as this is clearly AI generated and I don't feel that you've dig into the project. Sometimes your changes makes no sense and are a bit weird.

Maybe you're just learning software engineering and I'm really sorry in this case.

Also please don't resolve comments that are questions without answering to the questions. We'll see them haha 😉

olisaagbafor · 2025-01-13T23:07:38Z

@EwenQuim ok, I will take them gradually and one after another, thank you for the feedback, honestly I appreciate them 🙏

olisaagbafor · 2025-01-14T12:33:26Z

@EwenQuim please review ...

EwenQuim · 2025-01-15T10:02:02Z

Thanks for your PR! 🥳

What was your AI?

EwenQuim · 2025-01-15T10:31:07Z

mock_context.go

+// SetBody stores the provided body value for later retrieval
+func (m *MockContext[B]) SetBody(body B) {
+	m.body = body
+}


@ccoVeille @dylanhitt what do you think about it? That is not very Go style, is it?

What do you mean?

I'm sorry but you drag me here an without much context 😅

Are you talking about the fact to have a setter here?

I'm unsure what is the functionality or need this method try to achieve

Yeah I mean personally. I'm more used to the constructor of the mock setting all the needed values or as you said earlier just export the field.

Personally I expected this PR to be like if someone ran :GoImplements or whatever IDE's equivalent of that generation for all the needed func signatures. Maybe a few helpers. For instance just below this we have SetQueryParam that just sets UrlValues. This should also be updating OpenAPIParams in CommonContext if that's gonna be added in MockContext.

Actually wait a minute a call to QueryParam here isn't going to work. This implementation is just backpacking off of CommonContext's implementation. If you look at the code

func (c CommonContext[B]) QueryParam(name string) string { _, ok := c.OpenAPIParams[name] if !ok { slog.Warn("query parameter not expected in OpenAPI spec", "param", name, "expected_one_of", c.OpenAPIParams) } if !c.UrlValues.Has(name) { defaultValue, _ := c.OpenAPIParams[name].Default.(string) return defaultValue } return c.UrlValues.Get(name) }

Well I guess it will "work" but it's gonna log the warning every time and I don't believe the Default case will ever happen either I don't believe. All that being said I'm describing a lot of features for a Mock lib already I wouldn't expect that to be fully implemented in a first pass.

I guess to sum up my last part if you're gonna backpack off the functions that CommonContext already supplies, they should "work". For instance

func (m *MockContext[B]) HasQueryParam(key string) bool { _, exists := m.UrlValues[key] return exists }

above doesn't need to be implemented. CommonContext's implementation will suffice.

Thanks for the feedback!

From the points here, then I propose to

Make the fields public and remove unnecessary setters (more idiomatic Go)

Remove redundant methods that CommonContext already handles

Properly implement OpenAPI param handling

Should I go ahead and update the PR with these changes?

let me update them quickly

documentation/docs/guides/testing.md

ccoVeille · 2025-01-15T11:21:00Z

mock_context.go

+// SetBody stores the provided body value for later retrieval
+func (m *MockContext[B]) SetBody(body B) {
+	m.body = body
+}


What do you mean?

I'm sorry but you drag me here an without much context 😅

Are you talking about the fact to have a setter here?

I'm unsure what is the functionality or need this method try to achieve

documentation/docs/guides/testing.md

…text functionality

…bafor/fuego into feature/mock-context-for-testing

…bafor/fuego into Olisa

Convert private fields to public and update docs with examples

documentation/docs/guides/testing.md

mock_context.go

mock_context_test.go

Properly initialize CommonContext fields to prevent potential panics

…bafor/fuego into mmm

olisaagbafor · 2025-01-16T19:13:39Z

@dylanhitt please check

…validation

dylanhitt · 2025-01-17T19:39:53Z

I will when I find the time.

documentation/docs/guides/testing.md

olisaagbafor · 2025-01-29T12:52:44Z

good day @EwenQuim @dylanhitt @ccoVeille please am sorry for this long break. Just getting back from sick bed, am sorry for the long delays. Please, do I have any reservations on this?

…a request & controller with no body Changed WithXxx to SetXxx for query parameters. Removed the ...option pattern for declaring query parameters in the MockContext.

EwenQuim · 2025-01-31T16:25:48Z

Just getting back from sick bed, am sorry for the long delays

No problem, be safe!

I added some commits to go towards what I think is right and added some examples, what do you think about it @dylanhitt @ccoVeille?

dylanhitt · 2025-01-31T17:45:56Z

mock_context.go

+
+// Cookie returns a mock cookie
+func (m *MockContext[B]) Cookie(name string) (*http.Cookie, error) {
+	cookie, exists := m.Cookies[name]


Cookie needs it OpenAPIParam input

Ah I guess these are fine there. Unlike the other Params we don't really throw warning or anything here.

Yeah it will be useful to set default values for example. Because defaults are defined in the route registration, not the controller.
But they'll be defined in 2 places : route registration & tests and it would be weird because nothing guarantees that it will be the same in these 2 different places.

dylanhitt

I believe both Cookie and Header need their OpenAPIParam map entry. Could be wrong though

dylanhitt · 2025-01-31T17:46:20Z

mock_context.go

+
+// SetHeader sets a header in the mock context
+func (m *MockContext[B]) SetHeader(key, value string) {
+	m.Headers.Set(key, value)


Same - need it's associated Param

It's not necessary to have the available fields and methods section in the testing guide, as it's already documented in the codebase & the godoc reference.

EwenQuim · 2025-02-01T09:12:09Z

Thank you @olisaagbafor for this PR!

olisaagbafor · 2025-02-01T09:25:08Z

Thank you @EwenQuim @dylanhitt @ccoVeille for your utmost support here.
much appreciations!!!

feat: add framework-agnostic mock context for testing

dc235ba

Implements a MockContext[B] type that allows testing controllers without framework dependencies. Includes comprehensive tests and documentation.

dylanhitt reviewed Jan 13, 2025

View reviewed changes

README.md Outdated Show resolved Hide resolved

EwenQuim requested changes Jan 13, 2025

View reviewed changes

olisaagbafor marked this pull request as draft January 13, 2025 19:18

ccoVeille approved these changes Jan 13, 2025

View reviewed changes

olisaagbafor marked this pull request as ready for review January 13, 2025 19:25

EwenQuim requested changes Jan 13, 2025

View reviewed changes

TESTING.md Outdated Show resolved Hide resolved

TESTING.md Outdated Show resolved Hide resolved

TESTING.md Outdated Show resolved Hide resolved

TESTING.md Outdated Show resolved Hide resolved

TESTING.md Outdated Show resolved Hide resolved

olisaagbafor marked this pull request as draft January 14, 2025 08:52

olisaagbafor added 3 commits January 14, 2025 13:05

Resolved the review comments

6693115

Merge branch 'workflow' into feature/mock-context-for-testing

6ab9051

Removed the TESTING.md file

50d594c

olisaagbafor marked this pull request as ready for review January 14, 2025 12:23

Documentation and testing updates

13dda83

EwenQuim force-pushed the feature/mock-context-for-testing branch from fb4ca5d to 13dda83 Compare January 15, 2025 10:05

Merge branch 'main' into pr/olisaagbafor/351

3cea539

EwenQuim approved these changes Jan 15, 2025

View reviewed changes

EwenQuim reviewed Jan 15, 2025

View reviewed changes

Use internal.CommonContext to provide most functions in MockContext

f051446

ccoVeille reviewed Jan 15, 2025

View reviewed changes

olisaagbafor added 5 commits January 16, 2025 13:33

refactor(mock): make MockContext fields public and leverage CommonCon…

e3237c3

…text functionality

Merge branch 'main' into feature/mock-context-for-testing

110137e

Merge branch 'dev-' into feature/mock-context-for-testing

4792732

Merge branch 'feature/mock-context-for-testing' of github.com:olisaag…

20732df

…bafor/fuego into feature/mock-context-for-testing

Merge branch 'feature/mock-context-for-testing' of github.com:olisaag…

c2f039b

…bafor/fuego into Olisa

olisaagbafor added 2 commits January 16, 2025 18:40

feat(testing): Expose MockContext fields for simpler test setup

488feb9

Convert private fields to public and update docs with examples

Merge branch 'Olisa' into feature/mock-context-for-testing

08e3251

dylanhitt reviewed Jan 16, 2025

View reviewed changes

documentation/docs/guides/testing.md Show resolved Hide resolved

dylanhitt reviewed Jan 16, 2025

View reviewed changes

mock_context.go Show resolved Hide resolved

dylanhitt reviewed Jan 16, 2025

View reviewed changes

mock_context_test.go Outdated Show resolved Hide resolved

olisaagbafor added 3 commits January 16, 2025 19:43

fix(mock): Initialize OpenAPIParams in MockContext constructor

d8c2637

Properly initialize CommonContext fields to prevent potential panics

Merge branch 'feature/mock-context-for-testing' of github.com:olisaag…

5383101

…bafor/fuego into mmm

Removed unused import

bc99b13

feat(mock): Add helper methods for setting query params with OpenAPI …

50e87a7

…validation

EwenQuim reviewed Jan 20, 2025

View reviewed changes

documentation/docs/guides/testing.md Outdated Show resolved Hide resolved

EwenQuim added this to the v0.18 milestone Jan 21, 2025

EwenQuim added 4 commits January 29, 2025 20:39

Merge branch 'main' into pr/olisaagbafor/351

ccfa704

Removed unused comment

8357ed8

Linting

be830f5

Adds NewMockContextNoBody to create a new MockContext suitable for …

f2ad6cc

…a request & controller with no body Changed WithXxx to SetXxx for query parameters. Removed the ...option pattern for declaring query parameters in the MockContext.

dylanhitt reviewed Jan 31, 2025

View reviewed changes

dylanhitt approved these changes Jan 31, 2025

View reviewed changes

Removed the available fields and methods section from the testing guide

97f21ef

It's not necessary to have the available fields and methods section in the testing guide, as it's already documented in the codebase & the godoc reference.

EwenQuim merged commit b2965ee into go-fuego:main Feb 1, 2025
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add framework-agnostic mock context for testing #351

feat: add framework-agnostic mock context for testing #351

olisaagbafor commented Jan 13, 2025 •

edited

Loading

EwenQuim Jan 13, 2025

olisaagbafor Jan 14, 2025

EwenQuim commented Jan 13, 2025 •

edited

Loading

olisaagbafor commented Jan 13, 2025

olisaagbafor commented Jan 14, 2025

EwenQuim commented Jan 15, 2025 •

edited

Loading

EwenQuim Jan 15, 2025

ccoVeille Jan 15, 2025

dylanhitt Jan 15, 2025

dylanhitt Jan 15, 2025

olisaagbafor Jan 15, 2025

olisaagbafor Jan 15, 2025

ccoVeille Jan 15, 2025

olisaagbafor commented Jan 16, 2025

dylanhitt commented Jan 17, 2025

olisaagbafor commented Jan 29, 2025

EwenQuim commented Jan 31, 2025

dylanhitt Jan 31, 2025

EwenQuim Jan 31, 2025

dylanhitt Jan 31, 2025

EwenQuim Feb 1, 2025

dylanhitt left a comment •

edited

Loading

dylanhitt Jan 31, 2025

EwenQuim commented Feb 1, 2025

olisaagbafor commented Feb 1, 2025

feat: add framework-agnostic mock context for testing #351

feat: add framework-agnostic mock context for testing #351

Conversation

olisaagbafor commented Jan 13, 2025 • edited Loading

Add MockContext for testing controllers

Changes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EwenQuim commented Jan 13, 2025 • edited Loading

olisaagbafor commented Jan 13, 2025

olisaagbafor commented Jan 14, 2025

EwenQuim commented Jan 15, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olisaagbafor commented Jan 16, 2025

dylanhitt commented Jan 17, 2025

olisaagbafor commented Jan 29, 2025

EwenQuim commented Jan 31, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dylanhitt left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EwenQuim commented Feb 1, 2025

olisaagbafor commented Feb 1, 2025

olisaagbafor commented Jan 13, 2025 •

edited

Loading

EwenQuim commented Jan 13, 2025 •

edited

Loading

EwenQuim commented Jan 15, 2025 •

edited

Loading

dylanhitt left a comment •

edited

Loading