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

Caddy2 fix tests #109

Merged
merged 2 commits into from
Oct 29, 2023
Merged

Caddy2 fix tests #109

merged 2 commits into from
Oct 29, 2023

Conversation

johnzhanghua
Copy link
Contributor

1. What does this change do, exactly?

This change based on the PR 74 , to fix the broken test cases

2. Please link to the relevant issues.

PR 74 tests failed

3. Which documentation changes (if any) need to be made because of this PR?

4. Checklist

  • [ x] I have written tests and verified that they fail without my change
  • [ x] I made pull request as minimal and simple as possible. If change is not small or additional dependencies are required, I opened an issue to propose and discuss the design first
  • [x ] I have squashed any insignificant commits
  • [ x] This change has comments for package types, values, functions, and non-obvious lines of code

@gaby
Copy link
Collaborator

gaby commented Sep 25, 2023

@johnzhanghua Thanks for the PR, some of the changes introduced this issue:

[/home/runner/work/forwardproxy/forwardproxy/forwardproxy.go:397] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
    396: 	}
  > 397: 	r.Body.Close()
    398: 

@johnzhanghua
Copy link
Contributor Author

@johnzhanghua Thanks for the PR, some of the changes introduced this issue:

[/home/runner/work/forwardproxy/forwardproxy/forwardproxy.go:397] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
    396: 	}
  > 397: 	r.Body.Close()
    398: 

Sorry, fixed.

@gaby
Copy link
Collaborator

gaby commented Sep 25, 2023

@johnzhanghua GoSec issues are gone, some of the tests still fail

@gaby
Copy link
Collaborator

gaby commented Sep 25, 2023

@johnzhanghua You can run them locally using go test -v ./... with go 1.21

@johnzhanghua
Copy link
Contributor Author

@johnzhanghua You can run them locally using go test -v ./... with go 1.21

Added a wait, should be fine now. Tested locally with Docker ubuntu. Could you please trigger the checks ?

@gaby
Copy link
Collaborator

gaby commented Sep 26, 2023

@johnzhanghua Awesome, will start reviewing

@gaby gaby mentioned this pull request Oct 21, 2023
4 tasks
@gaby gaby merged commit 65d0590 into caddyserver:caddy2 Oct 29, 2023
6 checks passed
@gaby
Copy link
Collaborator

gaby commented Oct 29, 2023

@johnzhanghua Thank you for the PR, I will keep working on the caddy2 branch

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.

2 participants