oonimkall: remove support for {Bouncer,Collector}BaseURL

After this change, we're unable to test that the code WAIs when we cannot discover backends and open a report. This will be fixed once we have support for ProbeServicesBaseURL inside of the session.go file. Part of #621.
ooni · May 27, 2020 · 87fc000 · 87fc000
1 parent 871691d
commit 87fc000
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 81 deletions.
diff --git a/oonimkall/runner.go b/oonimkall/runner.go
@@ -63,9 +63,15 @@ func (r *runner) hasUnsupportedSettings(logger *chanLogger) (unsupported bool) {
 	if r.settings.Options.Backend != "" {
 		sadly("Options.Backend: not supported")
 	}
+	if r.settings.Options.BouncerBaseURL != "" {
+		sadly("Options.BouncerBaseURL: not supported")
+	}
 	if r.settings.Options.CABundlePath != "" {
 		logger.Warn("Options.CABundlePath: not supported")
 	}
+	if r.settings.Options.CollectorBaseURL != "" {
+		sadly("Options.CollectorBaseURL: not supported")
+	}
 	if r.settings.Options.ConstantBitrate != nil {
 		logger.Warn("Options.ConstantBitrate: not supported")
 	}
@@ -170,18 +176,6 @@ func (r *runner) newsession(logger *chanLogger) (*engine.Session, error) {
 		SoftwareVersion: r.settings.Options.SoftwareVersion,
 		TempDir:         r.settings.TempDir,
 	}
-	if r.settings.Options.BouncerBaseURL != "" {
-		config.AvailableBouncers = []model.Service{{
-			Address: r.settings.Options.BouncerBaseURL,
-			Type:    "https",
-		}}
-	}
-	if r.settings.Options.CollectorBaseURL != "" {
-		config.AvailableCollectors = []model.Service{{
-			Address: r.settings.Options.CollectorBaseURL,
-			Type:    "https",
-		}}
-	}
 	return engine.NewSession(config)
 }
 
@@ -239,6 +233,7 @@ func (r *runner) Run(ctx context.Context) {
 	if !r.settings.Options.NoBouncer {
 		logger.Info("Looking up OONI backends... please, be patient")
 		if err := sess.MaybeLookupBackends(); err != nil {
+			// TODO(bassosimone): we need to test this error case
 			r.emitter.EmitFailureStartup(err.Error())
 			return
 		}
@@ -295,6 +290,7 @@ func (r *runner) Run(ctx context.Context) {
 	if !r.settings.Options.NoCollector {
 		logger.Info("Opening report... please, be patient")
 		if err := experiment.OpenReport(); err != nil {
+			// TODO(bassosimone): we need to test this error case
 			r.emitter.EmitFailureGeneric(failureReportCreate, err.Error())
 			return
 		}

diff --git a/oonimkall/runner_test.go b/oonimkall/runner_test.go
@@ -22,7 +22,9 @@ func TestUnitRunnerHasUnsupportedSettings(t *testing.T) {
 		Options: settingsOptions{
 			AllEndpoints:          &falsebool,
 			Backend:               "foo",
+			BouncerBaseURL:        "https://ps.ooni.io/",
 			CABundlePath:          "foo",
+			CollectorBaseURL:      "https://ps.ooni.io/",
 			ConstantBitrate:       &falsebool,
 			DNSNameserver:         &emptystring,
 			DNSEngine:             &emptystring,
@@ -80,7 +82,7 @@ func TestUnitRunnerHasUnsupportedSettings(t *testing.T) {
 			log.Fatalf("invalid key: %s", ev.Key)
 		}
 	}
-	const expected = 31
+	const expected = 33
 	if len(seen) != expected {
 		t.Fatalf("expected: %d; seen %+v", expected, seen)
 	}

diff --git a/oonimkall/settings.go b/oonimkall/settings.go
@@ -66,7 +66,9 @@ type settingsOptions struct {
 	// to set it will cause a startup error.
 	Backend string `json:"backend,omitempty"`
 
-	// BouncerBaseURL contains the bouncer base URL
+	// BouncerBaseURL contains the bouncer base URL. This option is not
+	// implemented by this library. Attempting to set it will cause
+	// a startup failure.
 	BouncerBaseURL string `json:"bouncer_base_url,omitempty"`
 
 	// CABundlePath contains the CA bundle path. This
@@ -75,7 +77,9 @@ type settingsOptions struct {
 	// library will otherwise ignore this setting.
 	CABundlePath string `json:"net/ca_bundle_path,omitempty"`
 
-	// CollectorBaseURL contains the collector base URL
+	// CollectorBaseURL contains the collector base URL. This option is not
+	// implemented by this library. Attempting to set it will cause
+	// a startup failure.
 	CollectorBaseURL string `json:"collector_base_url,omitempty"`
 
 	// ConstantBitrate was an option for the DASH experiment that

diff --git a/oonimkall/task_test.go b/oonimkall/task_test.go
@@ -251,39 +251,6 @@ func TestIntegrationUnknownExperiment(t *testing.T) {
 	}
 }
 
-func TestIntegrationInvalidBouncerBaseURL(t *testing.T) {
-	task, err := oonimkall.StartTask(`{
-		"assets_dir": "../testdata/oonimkall/assets",
-		"log_level": "DEBUG",
-		"name": "Example",
-		"options": {
-			"bouncer_base_url": "\t",
-			"software_name": "oonimkall-test",
-			"software_version": "0.1.0"
-		},
-		"state_dir": "../testdata/oonimkall/state",
-		"temp_dir": "../testdata/oonimkall/tmp"
-	}`)
-	if err != nil {
-		t.Fatal(err)
-	}
-	var seen bool
-	for !task.IsDone() {
-		eventstr := task.WaitForNextEvent()
-		var event eventlike
-		if err := json.Unmarshal([]byte(eventstr), &event); err != nil {
-			t.Fatal(err)
-		}
-		if event.Key == "failure.startup" {
-			seen = true
-		}
-		t.Logf("%+v", event)
-	}
-	if !seen {
-		t.Fatal("did not see failure.startup")
-	}
-}
-
 func TestIntegrationInconsistentGeoIPSettings(t *testing.T) {
 	task, err := oonimkall.StartTask(`{
 		"assets_dir": "../testdata/oonimkall/assets",
@@ -350,39 +317,6 @@ func TestIntegrationInputIsRequired(t *testing.T) {
 	}
 }
 
-func TestIntegrationBadCollectorURL(t *testing.T) {
-	task, err := oonimkall.StartTask(`{
-		"assets_dir": "../testdata/oonimkall/assets",
-		"log_level": "DEBUG",
-		"name": "Example",
-		"options": {
-			"collector_base_url": "\t",
-			"software_name": "oonimkall-test",
-			"software_version": "0.1.0"
-		},
-		"state_dir": "../testdata/oonimkall/state",
-		"temp_dir": "../testdata/oonimkall/tmp"
-	}`)
-	if err != nil {
-		t.Fatal(err)
-	}
-	var seen bool
-	for !task.IsDone() {
-		eventstr := task.WaitForNextEvent()
-		var event eventlike
-		if err := json.Unmarshal([]byte(eventstr), &event); err != nil {
-			t.Fatal(err)
-		}
-		if event.Key == "failure.report_create" {
-			seen = true
-		}
-		t.Logf("%+v", event)
-	}
-	if !seen {
-		t.Fatal("did not see failure.report_create")
-	}
-}
-
 func TestIntegrationMaxRuntime(t *testing.T) {
 	begin := time.Now()
 	task, err := oonimkall.StartTask(`{