Skip to content

The usage of NewRequestWithValues(t, "PATCH", url, data) doesn't work in tests #29514

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
yp05327 opened this issue Mar 1, 2024 · 10 comments · Fixed by #29674
Closed

The usage of NewRequestWithValues(t, "PATCH", url, data) doesn't work in tests #29514

yp05327 opened this issue Mar 1, 2024 · 10 comments · Fixed by #29674
Labels

Comments

@yp05327
Copy link
Contributor

yp05327 commented Mar 1, 2024

Description

PATCH request will read data from Form, but NewRequestWithValues will insert data into Body, so these data will not be readed correctly.
As title, the usage of NewRequestWithValues(t, "PATCH", url, data) doesn't work in tests
image

e.g.
image
image
image

Gitea Version

latest

Can you reproduce the bug on the Gitea demo site?

No

Log Gist

No response

Screenshots

No response

Git Version

No response

Operating System

No response

How are you running Gitea?

build

Database

None

@yp05327 yp05327 changed the title The usage of `NewRequestWithValues(t, "PATCH", url, data) is incorrect in tests The usage of `NewRequestWithValues(t, "PATCH", url, data) doesn't work in tests Mar 1, 2024
@yp05327 yp05327 changed the title The usage of `NewRequestWithValues(t, "PATCH", url, data) doesn't work in tests The usage of NewRequestWithValues(t, "PATCH", url, data) doesn't work in tests Mar 1, 2024
@yp05327
Copy link
Contributor Author

yp05327 commented Mar 1, 2024

image
image
theObj is empty, so in this function, we will always set empty data. But ctx.Req.Form has the data, so maybe something changed?

@yp05327
Copy link
Contributor Author

yp05327 commented Mar 1, 2024

NewRequestWithJSON works but NewRequestWithValues doesn't work.

@yp05327
Copy link
Contributor Author

yp05327 commented Mar 1, 2024

For Form, if the type of the field is *string, it doesn't work, but if it is string, it works.

@yp05327
Copy link
Contributor Author

yp05327 commented Mar 1, 2024

@lunny
It seems that this issue is because gitea.com/go-chi/binding doesn't support non-struct pointer.
https://gitea.com/go-chi/binding/pulls/5

@yp05327
Copy link
Contributor Author

yp05327 commented Mar 8, 2024

Any plan to finish it, API requests with form type body can not be applied correctly which looks like a big bug.

@lunny
Copy link
Member

lunny commented Mar 8, 2024

https://gitea.com/go-chi/binding/pulls/5 merged.

@yp05327
Copy link
Contributor Author

yp05327 commented Mar 8, 2024

Then the next step is adding result checking in theses tests to confirm whether it works.

@lunny
Copy link
Member

lunny commented Mar 8, 2024

Can you help to send a PR?

@yp05327
Copy link
Contributor Author

yp05327 commented Mar 8, 2024

It seems that it does not work. Still got empty items.
I will send a PR to test it first, then let's fix it.

@yp05327
Copy link
Contributor Author

yp05327 commented Mar 21, 2024

@lunny
the origin PR is not enough, created a new one to fix this issue:
https://gitea.com/go-chi/binding/pulls/16

lunny pushed a commit that referenced this issue May 5, 2024
Fix #29514
There are too many usage of `NewRequestWithValues`, so there's no need
to check all of them.
Just one is enough I think.
GiteaBot pushed a commit to GiteaBot/gitea that referenced this issue May 5, 2024
Fix go-gitea#29514
There are too many usage of `NewRequestWithValues`, so there's no need
to check all of them.
Just one is enough I think.
lunny pushed a commit that referenced this issue May 5, 2024
Backport #29674 by @yp05327

Fix #29514
there are too many usage of `NewRequestWithValues`, so there's no need
to check all of them.
Just one is enough I think.

Co-authored-by: yp05327 <[email protected]>
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Aug 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants