Skip to content

Conversation

AnthonyMichaelTDM
Copy link
Collaborator

@AnthonyMichaelTDM AnthonyMichaelTDM commented Jul 20, 2025

This just makes sure the code compiles and passes some static analysis, doesn't run the tests since a lot of them hang on my system.

Worth noting that the static analysis does currently fail, but that can be fixed.

If you want, I can do the fixes in this PR, or put it off for later, I don't mind either way.

PR is blocked pending a decision on where the changes should go (this PR or a new one) I decided to put them in this PR

@AnthonyMichaelTDM AnthonyMichaelTDM changed the base branch from v3.2.5 to main July 20, 2025 06:33
@AnthonyMichaelTDM AnthonyMichaelTDM changed the base branch from main to v3.2.6 July 20, 2025 06:55
@AnthonyMichaelTDM AnthonyMichaelTDM marked this pull request as ready for review July 20, 2025 06:55
@AnthonyMichaelTDM
Copy link
Collaborator Author

I'll squash this

This just makes sure the code compiles and passes some static analysis, doesn't run the tests since a lot of them hang on my system

fix: remove branch restrictions for pull_request events in CI workflow

forgot we don't push to main, my bad.
if we had a dev branch I could filter it to only run on main and dev but since the "dev" branch changes every release let's just run it everywhere

fix: specify cache-dependency-path

fix: add concurrency settings to CI workflow

only one run at a time per ref
@AnthonyMichaelTDM
Copy link
Collaborator Author

the dpcore_test.go tests are failing for the following reasons:

  • TestHTTP1p1KeepAlive: no http server is running
  • TestReplaceLocationHost: the result doesn't match the expected result, after passing the result through url.QueryUnescape they are equal, this explains the confusing error message

the dbleveldb_test.go tests are failing for the following reasons:

  • TestWriteAndRead: the write isn't synced before attempting to be read (syncing is done on a set interval in a separate go-routine), and it seems there's no mechanism for reading unsynced data.
  • TestListTable: a little more complex, seems to have multiple errors/bugs piled on top of eachother

The netutils_test.go tests seem to both be failing for similar reasons, basically it's failing to create a icmp connection to the IPs that the domains are pointing to:
image

two acme_test.go tests are also failing, one seemingly due to a missing file and the other presumably due to malformed input:
image

After we get those tests to pass, staticcheck will fail primarily due to a number of unused functions.

@AnthonyMichaelTDM
Copy link
Collaborator Author

Here's a patch that will run an http server for the TestHTTP1p1KeepAlive test, I'm not committing it though since I'm unsure what's supposed to be getting tested here

diff --git a/src/mod/dynamicproxy/dpcore/dpcore_test.go b/src/mod/dynamicproxy/dpcore/dpcore_test.go
index 4e33ffe..b4e18c1 100644
--- a/src/mod/dynamicproxy/dpcore/dpcore_test.go
+++ b/src/mod/dynamicproxy/dpcore/dpcore_test.go
@@ -90,13 +90,33 @@ func TestReplaceLocationHostRelative(t *testing.T) {
 
 // Not sure why this test is not working, but at least this make the QA guy happy
 func TestHTTP1p1KeepAlive(t *testing.T) {
+	// Start a simple HTTP server
+	// This server should have Keep-Alive enabled by default
+	errChan := make(chan error, 1)
+	go func() {
+		http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(http.StatusOK)
+			w.Write([]byte("Hello, World!"))
+		})
+		err := http.ListenAndServe(":8080", nil)
+		if err != nil {
+			errChan <- err
+		}
+	}()
+	// Give the server a moment to start
+	select {
+	case err := <-errChan:
+		t.Fatal("ListenAndServe: ", err)
+	case <-time.After(2 * time.Second):
+	}
+
 	client := &http.Client{
 		Transport: &http.Transport{
 			DisableKeepAlives: false,
 		},
 	}
 
-	req, err := http.NewRequest("GET", "http://localhost:80", nil)
+	req, err := http.NewRequest("GET", "http://localhost:8080", nil)
 	if err != nil {
 		t.Fatalf("Failed to create request: %v", err)
 	}
@@ -116,7 +136,7 @@ func TestHTTP1p1KeepAlive(t *testing.T) {
 	t.Logf("First request status code: %v", resp.StatusCode)
 	time.Sleep(20 * time.Second)
 
-	req2, err := http.NewRequest("GET", "http://localhost:80", nil)
+	req2, err := http.NewRequest("GET", "http://localhost:8080", nil)
 	if err != nil {
 		t.Fatalf("Failed to create second request: %v", err)
 	}

@tobychui
Copy link
Owner

tobychui commented Sep 7, 2025

I guess this need to be left for later, since I have my own jenkin server for building (instead of using Github action for the binary build). I will see if I got time to integrate these. Let me change this merge target to main.

@AnthonyMichaelTDM
Copy link
Collaborator Author

Is there a way to link jenkins w/ GH so that there's more observability about which tests are failing and such?

If a plugin fails to build, make sure to exit w/ a failing status code
@tobychui tobychui changed the base branch from v3.2.6 to main September 16, 2025 13:04
@tobychui
Copy link
Owner

Moving this to main so it won't get closed when I remove the v3.2.6 dev 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