refactor(oohelperd): better distinguish different helpers (#434)

Part of ooni/probe#1733
ooni · Aug 17, 2021 · ce854e8 · ce854e8
1 parent bef5b87
commit ce854e8
Show file tree

Hide file tree

Showing 23 changed files with 48 additions and 45 deletions.
diff --git a/internal/cmd/oohelperd/internal/dns.go → ...oohelperd/internal/webconnectivity/dns.go b/internal/cmd/oohelperd/internal/dns.go → ...oohelperd/internal/webconnectivity/dns.go
@@ -1,4 +1,4 @@
-package internal
+package webconnectivity
 
 import (
 	"context"

diff --git a/internal/cmd/oohelperd/internal/fake_test.go → ...erd/internal/webconnectivity/fake_test.go b/internal/cmd/oohelperd/internal/fake_test.go → ...erd/internal/webconnectivity/fake_test.go
@@ -1,4 +1,4 @@
-package internal
+package webconnectivity
 
 import (
 	"context"

diff --git a/internal/cmd/oohelperd/internal/http.go → ...ohelperd/internal/webconnectivity/http.go b/internal/cmd/oohelperd/internal/http.go → ...ohelperd/internal/webconnectivity/http.go
@@ -1,4 +1,4 @@
-package internal
+package webconnectivity
 
 import (
 	"context"

diff --git a/internal/cmd/oohelperd/internal/http_test.go → ...erd/internal/webconnectivity/http_test.go b/internal/cmd/oohelperd/internal/http_test.go → ...erd/internal/webconnectivity/http_test.go
@@ -1,4 +1,4 @@
-package internal_test
+package webconnectivity
 
 import (
 	"context"
@@ -7,16 +7,14 @@ import (
 	"strings"
 	"sync"
 	"testing"
-
-	"github.com/ooni/probe-cli/v3/internal/cmd/oohelperd/internal"
 )
 
 func TestHTTPDoWithInvalidURL(t *testing.T) {
 	ctx := context.Background()
 	wg := new(sync.WaitGroup)
-	httpch := make(chan internal.CtrlHTTPResponse, 1)
+	httpch := make(chan CtrlHTTPResponse, 1)
 	wg.Add(1)
-	go internal.HTTPDo(ctx, &internal.HTTPConfig{
+	go HTTPDo(ctx, &HTTPConfig{
 		Client:            http.DefaultClient,
 		Headers:           nil,
 		MaxAcceptableBody: 1 << 24,
@@ -36,11 +34,11 @@ func TestHTTPDoWithHTTPTransportFailure(t *testing.T) {
 	expected := errors.New("mocked error")
 	ctx := context.Background()
 	wg := new(sync.WaitGroup)
-	httpch := make(chan internal.CtrlHTTPResponse, 1)
+	httpch := make(chan CtrlHTTPResponse, 1)
 	wg.Add(1)
-	go internal.HTTPDo(ctx, &internal.HTTPConfig{
+	go HTTPDo(ctx, &HTTPConfig{
 		Client: &http.Client{
-			Transport: internal.FakeTransport{
+			Transport: FakeTransport{
 				Err: expected,
 			},
 		},

diff --git a/internal/cmd/oohelperd/internal/measure.go → ...lperd/internal/webconnectivity/measure.go b/internal/cmd/oohelperd/internal/measure.go → ...lperd/internal/webconnectivity/measure.go
@@ -1,4 +1,4 @@
-package internal
+package webconnectivity
 
 import (
 	"context"

diff --git a/...rnal/cmd/oohelperd/internal/tcpconnect.go → ...rd/internal/webconnectivity/tcpconnect.go b/...rnal/cmd/oohelperd/internal/tcpconnect.go → ...rd/internal/webconnectivity/tcpconnect.go
@@ -1,4 +1,4 @@
-package internal
+package webconnectivity
 
 import (
 	"context"

diff --git a/internal/cmd/oohelperd/internal/internal.go → ...ternal/webconnectivity/webconnectivity.go b/internal/cmd/oohelperd/internal/internal.go → ...ternal/webconnectivity/webconnectivity.go
@@ -1,4 +1,4 @@
-package internal
+package webconnectivity
 
 import (
 	"encoding/json"

diff --git a/...l/cmd/oohelperd/internal/internal_test.go → ...l/webconnectivity/webconnectivity_test.go b/...l/cmd/oohelperd/internal/internal_test.go → ...l/webconnectivity/webconnectivity_test.go
@@ -1,4 +1,4 @@
-package internal_test
+package webconnectivity
 
 import (
 	"context"
@@ -10,7 +10,6 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/ooni/probe-cli/v3/internal/cmd/oohelperd/internal"
 	"github.com/ooni/probe-cli/v3/internal/iox"
 	"github.com/ooni/probe-cli/v3/internal/netxlite"
 )
@@ -52,7 +51,7 @@ const requestWithoutDomainName = `{
 }`
 
 func TestWorkingAsIntended(t *testing.T) {
-	handler := internal.Handler{
+	handler := Handler{
 		Client:            http.DefaultClient,
 		Dialer:            new(net.Dialer),
 		MaxAcceptableBody: 1 << 24,
@@ -144,15 +143,15 @@ func TestWorkingAsIntended(t *testing.T) {
 
 func TestHandlerWithRequestBodyReadingError(t *testing.T) {
 	expected := errors.New("mocked error")
-	handler := internal.Handler{MaxAcceptableBody: 1 << 24}
-	rw := internal.NewFakeResponseWriter()
+	handler := Handler{MaxAcceptableBody: 1 << 24}
+	rw := NewFakeResponseWriter()
 	req := &http.Request{
 		Method: "POST",
 		Header: map[string][]string{
 			"Content-Type":   {"application/json"},
 			"Content-Length": {"2048"},
 		},
-		Body: &internal.FakeBody{Err: expected},
+		Body: &FakeBody{Err: expected},
 	}
 	handler.ServeHTTP(rw, req)
 	if rw.StatusCode != 400 {

diff --git a/...l/cmd/oohelperd/internal/nwcth/explore.go → ...md/oohelperd/internal/websteps/explore.go b/...l/cmd/oohelperd/internal/nwcth/explore.go → ...md/oohelperd/internal/websteps/explore.go
@@ -1,4 +1,4 @@
-package nwcth
+package websteps
 
 import (
 	"crypto/tls"

diff --git a/.../oohelperd/internal/nwcth/explore_test.go → ...helperd/internal/websteps/explore_test.go b/.../oohelperd/internal/nwcth/explore_test.go → ...helperd/internal/websteps/explore_test.go
@@ -1,4 +1,4 @@
-package nwcth
+package websteps
 
 import (
 	"net/http"
@@ -56,6 +56,7 @@ func TestExploreSuccessWithH3(t *testing.T) {
 
 func TestGetSuccess(t *testing.T) {
 	u, err := url.Parse("https://example.com")
+	runtimex.PanicOnError(err, "url.Parse failed for clearly good URL")
 	resp, err := explorer.get(u, nil)
 	if err != nil {
 		t.Fatal("unexpected error")
@@ -72,6 +73,7 @@ func TestGetSuccess(t *testing.T) {
 
 func TestGetFailure(t *testing.T) {
 	u, err := url.Parse("https://example.example")
+	runtimex.PanicOnError(err, "url.Parse failed for clearly good URL")
 	resp, err := explorer.get(u, nil)
 	if err == nil {
 		t.Fatal("expected an error here")
@@ -83,6 +85,7 @@ func TestGetFailure(t *testing.T) {
 
 func TestGetH3Success(t *testing.T) {
 	u, err := url.Parse("https://www.google.com")
+	runtimex.PanicOnError(err, "url.Parse failed for clearly good URL")
 	h3u := &h3URL{URL: u, proto: "h3"}
 	resp, err := explorer.getH3(h3u, nil)
 	if err != nil {
@@ -100,6 +103,7 @@ func TestGetH3Success(t *testing.T) {
 
 func TestGetH3Failure(t *testing.T) {
 	u, err := url.Parse("https://www.google.google")
+	runtimex.PanicOnError(err, "url.Parse failed for clearly good URL")
 	h3u := &h3URL{URL: u, proto: "h3"}
 	resp, err := explorer.getH3(h3u, nil)
 	if err == nil {

diff --git a/...l/cmd/oohelperd/internal/nwcth/factory.go → ...md/oohelperd/internal/websteps/factory.go b/...l/cmd/oohelperd/internal/nwcth/factory.go → ...md/oohelperd/internal/websteps/factory.go
@@ -1,4 +1,4 @@
-package nwcth
+package websteps
 
 import (
 	"context"

diff --git a/.../cmd/oohelperd/internal/nwcth/generate.go → ...d/oohelperd/internal/websteps/generate.go b/.../cmd/oohelperd/internal/nwcth/generate.go → ...d/oohelperd/internal/websteps/generate.go
@@ -1,4 +1,4 @@
-package nwcth
+package websteps
 
 import (
 	"context"

diff --git a/...oohelperd/internal/nwcth/generate_test.go → ...elperd/internal/websteps/generate_test.go b/...oohelperd/internal/nwcth/generate_test.go → ...elperd/internal/websteps/generate_test.go
@@ -1,4 +1,4 @@
-package nwcth
+package websteps
 
 import (
 	"context"
@@ -75,8 +75,9 @@ func TestGenerateDNSFailure(t *testing.T) {
 
 func TestGenerate(t *testing.T) {
 	u, err := url.Parse("http://www.google.com")
+	runtimex.PanicOnError(err, "url.Parse failed for clearly good URL")
 	u2, err := url.Parse("https://www.google.com")
-	runtimex.PanicOnError(err, "url.Parse failed")
+	runtimex.PanicOnError(err, "url.Parse failed for clearly good URL")
 	rts := []*RoundTrip{
 		{
 			Proto: "http",
@@ -390,8 +391,9 @@ func TestGenerateHTTPDoFails(t *testing.T) {
 		resolver:  newResolver(),
 	}
 	u, err := url.Parse("http://www.google.com")
+	runtimex.PanicOnError(err, "url.Parse failed for clearly good URL")
 	u2, err := url.Parse("https://www.google.com")
-	runtimex.PanicOnError(err, "url.Parse failed")
+	runtimex.PanicOnError(err, "url.Parse failed for clearly good URL")
 	rts := []*RoundTrip{
 		{
 			Proto: "http",

diff --git a/internal/cmd/oohelperd/internal/nwcth/h3.go → ...nal/cmd/oohelperd/internal/websteps/h3.go b/internal/cmd/oohelperd/internal/nwcth/h3.go → ...nal/cmd/oohelperd/internal/websteps/h3.go
@@ -1,4 +1,4 @@
-package nwcth
+package websteps
 
 import (
 	"errors"

diff --git a/...l/cmd/oohelperd/internal/nwcth/h3_test.go → ...md/oohelperd/internal/websteps/h3_test.go b/...l/cmd/oohelperd/internal/nwcth/h3_test.go → ...md/oohelperd/internal/websteps/h3_test.go
@@ -1,4 +1,4 @@
-package nwcth
+package websteps
 
 import (
 	"net/http"

diff --git a/...oohelperd/internal/nwcth/initialchecks.go → ...elperd/internal/websteps/initialchecks.go b/...oohelperd/internal/nwcth/initialchecks.go → ...elperd/internal/websteps/initialchecks.go
@@ -1,4 +1,4 @@
-package nwcth
+package websteps
 
 import (
 	"context"

diff --git a/...perd/internal/nwcth/initialchecks_test.go → ...d/internal/websteps/initialchecks_test.go b/...perd/internal/nwcth/initialchecks_test.go → ...d/internal/websteps/initialchecks_test.go
@@ -1,4 +1,4 @@
-package nwcth
+package websteps
 
 import (
 	"testing"

diff --git a/...l/cmd/oohelperd/internal/nwcth/measure.go → ...md/oohelperd/internal/websteps/measure.go b/...l/cmd/oohelperd/internal/nwcth/measure.go → ...md/oohelperd/internal/websteps/measure.go
@@ -1,4 +1,4 @@
-package nwcth
+package websteps
 
 import (
 	"context"
@@ -17,7 +17,7 @@ type (
 	ControlResponse = websteps.ControlResponse
 )
 
-var ErrInternalServer = errors.New("Internal server failure")
+var ErrInternalServer = errors.New("internal server error")
 
 // Config contains the building blocks of the testhelper algorithm
 type Config struct {

diff --git a/.../oohelperd/internal/nwcth/measure_test.go → ...helperd/internal/websteps/measure_test.go b/.../oohelperd/internal/nwcth/measure_test.go → ...helperd/internal/websteps/measure_test.go
@@ -1,4 +1,4 @@
-package nwcth
+package websteps
 
 import (
 	"context"

diff --git a/...nal/cmd/oohelperd/internal/nwcth/model.go → .../cmd/oohelperd/internal/websteps/model.go b/...nal/cmd/oohelperd/internal/nwcth/model.go → .../cmd/oohelperd/internal/websteps/model.go
@@ -1,4 +1,4 @@
-package nwcth
+package websteps
 
 import (
 	"github.com/ooni/probe-cli/v3/internal/engine/experiment/websteps"

diff --git a/...nal/cmd/oohelperd/internal/nwcth/nwcth.go → ...d/oohelperd/internal/websteps/websteps.go b/...nal/cmd/oohelperd/internal/nwcth/nwcth.go → ...d/oohelperd/internal/websteps/websteps.go
@@ -1,7 +1,7 @@
-// Package nwcth implements the new web connectivity test helper.
+// Package websteps implements the websteps test helper.
 //
 // See https://github.com/ooni/spec/blob/master/backends/th-007-nwcth.md
-package nwcth
+package websteps
 
 import (
 	"encoding/json"
@@ -30,7 +30,7 @@ type Handler struct {
 }
 
 // ServeHTTP implements http.Handler.ServeHTTP.
-func (h Handler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
+func (h *Handler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 	w.Header().Add("Server", fmt.Sprintf(
 		"oohelperd/%s ooniprobe-engine/%s", version.Version, version.Version,
 	))

diff --git a/...md/oohelperd/internal/nwcth/nwcth_test.go → ...elperd/internal/websteps/websteps_test.go b/...md/oohelperd/internal/nwcth/nwcth_test.go → ...elperd/internal/websteps/websteps_test.go
@@ -1,4 +1,4 @@
-package nwcth
+package websteps
 
 import (
 	"context"
@@ -105,7 +105,7 @@ const requestWithoutDomainName = `{
 
 func TestWorkingAsIntended(t *testing.T) {
 	handler := Handler{Config: &Config{}}
-	srv := httptest.NewServer(handler)
+	srv := httptest.NewServer(&handler)
 	defer srv.Close()
 	type expectationSpec struct {
 		name            string
@@ -205,7 +205,7 @@ func TestWorkingAsIntended(t *testing.T) {
 
 func TestHandlerWithInternalServerError(t *testing.T) {
 	handler := Handler{Config: &Config{explorer: &MockExplorer{}}}
-	srv := httptest.NewServer(handler)
+	srv := httptest.NewServer(&handler)
 	defer srv.Close()
 	body := strings.NewReader(`{"url": "https://example.com"}`)
 	req, err := http.NewRequest("POST", srv.URL, body)

diff --git a/internal/cmd/oohelperd/oohelperd.go b/internal/cmd/oohelperd/oohelperd.go
@@ -9,8 +9,8 @@ import (
 	"time"
 
 	"github.com/apex/log"
-	"github.com/ooni/probe-cli/v3/internal/cmd/oohelperd/internal"
-	"github.com/ooni/probe-cli/v3/internal/cmd/oohelperd/internal/nwcth"
+	"github.com/ooni/probe-cli/v3/internal/cmd/oohelperd/internal/webconnectivity"
+	"github.com/ooni/probe-cli/v3/internal/cmd/oohelperd/internal/websteps"
 	"github.com/ooni/probe-cli/v3/internal/engine/netx"
 )
 
@@ -53,8 +53,8 @@ func main() {
 
 func testableMain() {
 	mux := http.NewServeMux()
-	mux.Handle("/api/unstable/nwcth", nwcth.Handler{Config: &nwcth.Config{}})
-	mux.Handle("/", internal.Handler{
+	mux.Handle("/api/unstable/websteps", &websteps.Handler{Config: &websteps.Config{}})
+	mux.Handle("/", webconnectivity.Handler{
 		Client:            httpx,
 		Dialer:            dialer,
 		MaxAcceptableBody: maxAcceptableBody,