From 58199a020ef1875060e927f5ca14f0307d46f17c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arturo=20Filast=C3=B2?= Date: Mon, 2 Dec 2019 15:15:50 +0200 Subject: [PATCH 01/10] Refactoring to reduce package count * Consolidate util and utils into the same package * Move internal/onboard into internal/cli/onboard * Move maybeOnboard into the onboard package --- internal/cli/onboard/onboard.go | 156 +++++++++++++++++++++- internal/cli/run/run.go | 3 +- internal/database/models.go | 2 +- internal/log/handlers/cli/cli.go | 8 +- internal/log/handlers/cli/measurements.go | 26 ++-- internal/log/handlers/cli/results.go | 22 +-- internal/onboard/onboard.go | 141 ------------------- internal/output/output.go | 6 +- ooni.go | 15 --- {internal => utils}/shutil/shutil.go | 0 {internal/util => utils}/util_test.go | 2 +- internal/util/util.go => utils/utils.go | 2 +- 12 files changed, 189 insertions(+), 194 deletions(-) delete mode 100644 internal/onboard/onboard.go rename {internal => utils}/shutil/shutil.go (100%) rename {internal/util => utils}/util_test.go (96%) rename internal/util/util.go => utils/utils.go (99%) diff --git a/internal/cli/onboard/onboard.go b/internal/cli/onboard/onboard.go index ddbe97f..ef55bb9 100644 --- a/internal/cli/onboard/onboard.go +++ b/internal/cli/onboard/onboard.go @@ -1,14 +1,164 @@ package onboard import ( - "errors" + "fmt" "github.com/alecthomas/kingpin" "github.com/apex/log" + "github.com/fatih/color" + ooni "github.com/ooni/probe-cli" + "github.com/ooni/probe-cli/config" "github.com/ooni/probe-cli/internal/cli/root" - "github.com/ooni/probe-cli/internal/onboard" + "github.com/ooni/probe-cli/internal/output" + "github.com/pkg/errors" + "gopkg.in/AlecAivazis/survey.v1" ) +// Onboarding start the interactive onboarding procedure +func Onboarding(config *config.Config) error { + output.SectionTitle("What is OONI Probe?") + + fmt.Println() + output.Paragraph("Your tool for detecting internet censorship!") + fmt.Println() + output.Paragraph("OONI Probe checks whether your provider blocks access to sites and services. Run OONI Probe to collect evidence of internet censorship and to measure your network performance.") + fmt.Println() + output.PressEnterToContinue("Press 'Enter' to continue...") + + output.SectionTitle("Heads Up") + fmt.Println() + output.Bullet("Anyone monitoring your internet activity (such as your government or ISP) may be able to see that you are running OONI Probe.") + fmt.Println() + output.Bullet("The network data you will collect will automatically be published (unless you opt-out in the settings).") + fmt.Println() + output.Bullet("You may test objectionable sites.") + fmt.Println() + output.Bullet("Read the documentation to learn more.") + fmt.Println() + output.PressEnterToContinue("Press 'Enter' to continue...") + + output.SectionTitle("Pop Quiz!") + output.Paragraph("") + answer := "" + quiz1 := &survey.Select{ + Message: "Anyone monitoring my internet activity may be able to see that I am running OONI Probe.", + Options: []string{"true", "false"}, + Default: "true", + } + survey.AskOne(quiz1, &answer, nil) + if answer != "true" { + output.Paragraph(color.RedString("Actually...")) + output.Paragraph("OONI Probe is not a privacy tool. Therefore, anyone monitoring your internet activity may be able to see which software you are running.") + } else { + output.Paragraph(color.BlueString("Good job!")) + } + answer = "" + quiz2 := &survey.Select{ + Message: "The network data I will collect will automatically be published (unless I opt-out in the settings).", + Options: []string{"true", "false"}, + Default: "true", + } + survey.AskOne(quiz2, &answer, nil) + if answer != "true" { + output.Paragraph(color.RedString("Actually...")) + output.Paragraph("The network data you will collect will automatically be published to increase transparency of internet censorship (unless you opt-out in the settings).") + } else { + output.Paragraph(color.BlueString("Well done!")) + } + + changeDefaults := false + prompt := &survey.Confirm{ + Message: "Do you want to change the default settings?", + Default: false, + } + survey.AskOne(prompt, &changeDefaults, nil) + + settings := struct { + IncludeIP bool + IncludeNetwork bool + IncludeCountry bool + UploadResults bool + SendCrashReports bool + }{} + settings.IncludeIP = false + settings.IncludeNetwork = true + settings.IncludeCountry = true + settings.UploadResults = true + settings.SendCrashReports = true + + if changeDefaults == true { + var qs = []*survey.Question{ + { + Name: "IncludeIP", + Prompt: &survey.Confirm{Message: "Should we include your IP?"}, + }, + { + Name: "IncludeNetwork", + Prompt: &survey.Confirm{ + Message: "Can we include your network name?", + Default: true, + }, + }, + { + Name: "IncludeCountry", + Prompt: &survey.Confirm{ + Message: "Can we include your country name?", + Default: true, + }, + }, + { + Name: "UploadResults", + Prompt: &survey.Confirm{ + Message: "Can we upload your results?", + Default: true, + }, + }, + { + Name: "SendCrashReports", + Prompt: &survey.Confirm{ + Message: "Can we send crash reports to OONI?", + Default: true, + }, + }, + } + + err := survey.Ask(qs, &settings) + if err != nil { + log.WithError(err).Error("there was an error in parsing your responses") + return err + } + } + + config.Lock() + config.InformedConsent = true + config.Sharing.IncludeCountry = settings.IncludeCountry + config.Advanced.SendCrashReports = settings.SendCrashReports + config.Sharing.IncludeIP = settings.IncludeIP + config.Sharing.IncludeASN = settings.IncludeNetwork + config.Sharing.UploadResults = settings.UploadResults + config.Unlock() + + if err := config.Write(); err != nil { + log.WithError(err).Error("failed to write config file") + return err + } + return nil +} + +// MaybeOnboarding will run the onboarding process only if the informed consent +// config option is set to false +func MaybeOnboarding(c *ooni.Context) error { + if c.Config.InformedConsent == false { + if c.IsBatch == true { + return errors.New("cannot run onboarding in batch mode") + } + if err := Onboarding(c.Config); err != nil { + return errors.Wrap(err, "onboarding") + } + } + return nil +} + func init() { cmd := root.Command("onboard", "Starts the onboarding process") @@ -35,6 +185,6 @@ func init() { return errors.New("cannot do onboarding in batch mode") } - return onboard.Onboarding(ctx.Config) + return Onboarding(ctx.Config) }) } diff --git a/internal/cli/run/run.go b/internal/cli/run/run.go index f27aecd..8090180 100644 --- a/internal/cli/run/run.go +++ b/internal/cli/run/run.go @@ -7,6 +7,7 @@ import ( "github.com/apex/log" "github.com/fatih/color" ooni "github.com/ooni/probe-cli" + "github.com/ooni/probe-cli/internal/cli/onboard" "github.com/ooni/probe-cli/internal/cli/root" "github.com/ooni/probe-cli/internal/database" "github.com/ooni/probe-cli/nettests" @@ -66,7 +67,7 @@ func init() { return err } - if err = ctx.MaybeOnboarding(); err != nil { + if err = onboard.MaybeOnboarding(ctx); err != nil { log.WithError(err).Error("failed to perform onboarding") return err } diff --git a/internal/database/models.go b/internal/database/models.go index 8e26de5..6ec6306 100644 --- a/internal/database/models.go +++ b/internal/database/models.go @@ -6,7 +6,7 @@ import ( "path/filepath" "time" - "github.com/ooni/probe-cli/internal/shutil" + "github.com/ooni/probe-cli/utils/shutil" "github.com/pkg/errors" "upper.io/db.v3/lib/sqlbuilder" ) diff --git a/internal/log/handlers/cli/cli.go b/internal/log/handlers/cli/cli.go index cd3e97e..5bb033a 100644 --- a/internal/log/handlers/cli/cli.go +++ b/internal/log/handlers/cli/cli.go @@ -11,7 +11,7 @@ import ( "github.com/apex/log" "github.com/fatih/color" colorable "github.com/mattn/go-colorable" - "github.com/ooni/probe-cli/internal/util" + "github.com/ooni/probe-cli/utils" ) // Default handler outputting to stderr. @@ -67,7 +67,7 @@ func logSectionTitle(w io.Writer, f log.Fields) error { title := f.Get("title").(string) fmt.Fprintf(w, "┏"+strings.Repeat("━", colWidth+2)+"┓\n") - fmt.Fprintf(w, "┃ %s ┃\n", util.RightPad(title, colWidth)) + fmt.Fprintf(w, "┃ %s ┃\n", utils.RightPad(title, colWidth)) fmt.Fprintf(w, "┗"+strings.Repeat("━", colWidth+2)+"┛\n") return nil } @@ -84,7 +84,7 @@ func logTable(w io.Writer, f log.Fields) error { continue } line := fmt.Sprintf("%s: %s", color.Sprint(name), f.Get(name)) - lineLength := util.EscapeAwareRuneCountInString(line) + lineLength := utils.EscapeAwareRuneCountInString(line) lines = append(lines, line) if colWidth < lineLength { colWidth = lineLength @@ -94,7 +94,7 @@ func logTable(w io.Writer, f log.Fields) error { fmt.Fprintf(w, "┏"+strings.Repeat("━", colWidth+2)+"┓\n") for _, line := range lines { fmt.Fprintf(w, "┃ %s ┃\n", - util.RightPad(line, colWidth), + utils.RightPad(line, colWidth), ) } fmt.Fprintf(w, "┗"+strings.Repeat("━", colWidth+2)+"┛\n") diff --git a/internal/log/handlers/cli/measurements.go b/internal/log/handlers/cli/measurements.go index a0ed0c7..fa27c75 100644 --- a/internal/log/handlers/cli/measurements.go +++ b/internal/log/handlers/cli/measurements.go @@ -9,7 +9,7 @@ import ( "time" "github.com/apex/log" - "github.com/ooni/probe-cli/internal/util" + "github.com/ooni/probe-cli/utils" ) func statusIcon(ok bool) string { @@ -35,7 +35,7 @@ func logTestKeys(w io.Writer, testKeys string) error { } for _, line := range testKeysLines { fmt.Fprintf(w, fmt.Sprintf("│ %s │\n", - util.RightPad(line, colWidth*2))) + utils.RightPad(line, colWidth*2))) } return nil } @@ -71,22 +71,22 @@ func logMeasurementItem(w io.Writer, f log.Fields) error { failureStr := fmt.Sprintf("success: %s", statusIcon(!isFailed)) fmt.Fprintf(w, fmt.Sprintf("│ %s │\n", - util.RightPad( + utils.RightPad( fmt.Sprintf("#%d", rID), colWidth*2))) if url != "" { fmt.Fprintf(w, fmt.Sprintf("│ %s │\n", - util.RightPad( + utils.RightPad( fmt.Sprintf("%s (%s)", url, urlCategoryCode), colWidth*2))) } fmt.Fprintf(w, fmt.Sprintf("│ %s %s│\n", - util.RightPad(testName, colWidth), - util.RightPad(anomalyStr, colWidth))) + utils.RightPad(testName, colWidth), + utils.RightPad(anomalyStr, colWidth))) fmt.Fprintf(w, fmt.Sprintf("│ %s %s│\n", - util.RightPad(failureStr, colWidth), - util.RightPad(uploadStr, colWidth))) + utils.RightPad(failureStr, colWidth), + utils.RightPad(uploadStr, colWidth))) if testKeys != "" { if err := logTestKeys(w, testKeys); err != nil { @@ -116,15 +116,15 @@ func logMeasurementSummary(w io.Writer, f log.Fields) error { networkName := f.Get("network_name").(string) fmt.Fprintf(w, " │ %s │\n", - util.RightPad(startTime.Format(time.RFC822), (colWidth+3)*3), + utils.RightPad(startTime.Format(time.RFC822), (colWidth+3)*3), ) fmt.Fprintf(w, " │ %s │\n", - util.RightPad(fmt.Sprintf("AS%d, %s (%s)", asn, networkName, countryCode), (colWidth+3)*3), + utils.RightPad(fmt.Sprintf("AS%d, %s (%s)", asn, networkName, countryCode), (colWidth+3)*3), ) fmt.Fprintf(w, " │ %s %s %s │\n", - util.RightPad(fmt.Sprintf("%.2fs", totalRuntime), colWidth), - util.RightPad(fmt.Sprintf("%d/%d anmls", anomalyCount, totalCount), colWidth), - util.RightPad(fmt.Sprintf("⬆ %s ⬇ %s", formatSize(dataUp), formatSize(dataDown)), colWidth+4)) + utils.RightPad(fmt.Sprintf("%.2fs", totalRuntime), colWidth), + utils.RightPad(fmt.Sprintf("%d/%d anmls", anomalyCount, totalCount), colWidth), + utils.RightPad(fmt.Sprintf("⬆ %s ⬇ %s", formatSize(dataUp), formatSize(dataDown)), colWidth+4)) fmt.Fprintf(w, " └────────────────────────────────────────────────┘\n") return nil diff --git a/internal/log/handlers/cli/results.go b/internal/log/handlers/cli/results.go index 2c367aa..f0d9f2a 100644 --- a/internal/log/handlers/cli/results.go +++ b/internal/log/handlers/cli/results.go @@ -9,7 +9,7 @@ import ( "github.com/apex/log" "github.com/ooni/probe-cli/internal/database" - "github.com/ooni/probe-cli/internal/util" + "github.com/ooni/probe-cli/utils" ) func formatSpeed(speed float64) string { @@ -95,7 +95,7 @@ func logResultItem(w io.Writer, f log.Fields) error { fmt.Fprintf(w, "┢"+strings.Repeat("━", colWidth*2+2)+"┪\n") } - firstRow := util.RightPad(fmt.Sprintf("#%d - %s", rID, startTime.Format(time.RFC822)), colWidth*2) + firstRow := utils.RightPad(fmt.Sprintf("#%d - %s", rID, startTime.Format(time.RFC822)), colWidth*2) fmt.Fprintf(w, "┃ "+firstRow+" ┃\n") fmt.Fprintf(w, "┡"+strings.Repeat("━", colWidth*2+2)+"┩\n") @@ -105,14 +105,14 @@ func logResultItem(w io.Writer, f log.Fields) error { f.Get("test_keys").(string)) fmt.Fprintf(w, fmt.Sprintf("│ %s %s│\n", - util.RightPad(name, colWidth), - util.RightPad(summary[0], colWidth))) + utils.RightPad(name, colWidth), + utils.RightPad(summary[0], colWidth))) fmt.Fprintf(w, fmt.Sprintf("│ %s %s│\n", - util.RightPad(networkName, colWidth), - util.RightPad(summary[1], colWidth))) + utils.RightPad(networkName, colWidth), + utils.RightPad(summary[1], colWidth))) fmt.Fprintf(w, fmt.Sprintf("│ %s %s│\n", - util.RightPad(asn, colWidth), - util.RightPad(summary[2], colWidth))) + utils.RightPad(asn, colWidth), + utils.RightPad(summary[2], colWidth))) if index == totalCount-1 { if isDone == true { @@ -139,9 +139,9 @@ func logResultSummary(w io.Writer, f log.Fields) error { } // └┬──────────────┬──────────────┬──────────────┬ fmt.Fprintf(w, " │ %s │ %s │ %s │\n", - util.RightPad(fmt.Sprintf("%d tests", tests), 12), - util.RightPad(fmt.Sprintf("%d nets", networks), 12), - util.RightPad(fmt.Sprintf("⬆ %s ⬇ %s", formatSize(dataUp), formatSize(dataDown)), 16)) + utils.RightPad(fmt.Sprintf("%d tests", tests), 12), + utils.RightPad(fmt.Sprintf("%d nets", networks), 12), + utils.RightPad(fmt.Sprintf("⬆ %s ⬇ %s", formatSize(dataUp), formatSize(dataDown)), 16)) fmt.Fprintf(w, " └──────────────┴──────────────┴──────────────────┘\n") return nil diff --git a/internal/onboard/onboard.go b/internal/onboard/onboard.go deleted file mode 100644 index 2144d0d..0000000 --- a/internal/onboard/onboard.go +++ /dev/null @@ -1,141 +0,0 @@ -package onboard - -import ( - "fmt" - - "github.com/apex/log" - "github.com/fatih/color" - "github.com/ooni/probe-cli/config" - "github.com/ooni/probe-cli/internal/output" - survey "gopkg.in/AlecAivazis/survey.v1" -) - -func Onboarding(config *config.Config) error { - output.SectionTitle("What is OONI Probe?") - - fmt.Println() - output.Paragraph("Your tool for detecting internet censorship!") - fmt.Println() - output.Paragraph("OONI Probe checks whether your provider blocks access to sites and services. Run OONI Probe to collect evidence of internet censorship and to measure your network performance.") - fmt.Println() - output.PressEnterToContinue("Press 'Enter' to continue...") - - output.SectionTitle("Heads Up") - fmt.Println() - output.Bullet("Anyone monitoring your internet activity (such as your government or ISP) may be able to see that you are running OONI Probe.") - fmt.Println() - output.Bullet("The network data you will collect will automatically be published (unless you opt-out in the settings).") - fmt.Println() - output.Bullet("You may test objectionable sites.") - fmt.Println() - output.Bullet("Read the documentation to learn more.") - fmt.Println() - output.PressEnterToContinue("Press 'Enter' to continue...") - - output.SectionTitle("Pop Quiz!") - output.Paragraph("") - answer := "" - quiz1 := &survey.Select{ - Message: "Anyone monitoring my internet activity may be able to see that I am running OONI Probe.", - Options: []string{"true", "false"}, - Default: "true", - } - survey.AskOne(quiz1, &answer, nil) - if answer != "true" { - output.Paragraph(color.RedString("Actually...")) - output.Paragraph("OONI Probe is not a privacy tool. Therefore, anyone monitoring your internet activity may be able to see which software you are running.") - } else { - output.Paragraph(color.BlueString("Good job!")) - } - answer = "" - quiz2 := &survey.Select{ - Message: "The network data I will collect will automatically be published (unless I opt-out in the settings).", - Options: []string{"true", "false"}, - Default: "true", - } - survey.AskOne(quiz2, &answer, nil) - if answer != "true" { - output.Paragraph(color.RedString("Actually...")) - output.Paragraph("The network data you will collect will automatically be published to increase transparency of internet censorship (unless you opt-out in the settings).") - } else { - output.Paragraph(color.BlueString("Well done!")) - } - - changeDefaults := false - prompt := &survey.Confirm{ - Message: "Do you want to change the default settings?", - Default: false, - } - survey.AskOne(prompt, &changeDefaults, nil) - - settings := struct { - IncludeIP bool - IncludeNetwork bool - IncludeCountry bool - UploadResults bool - SendCrashReports bool - }{} - settings.IncludeIP = false - settings.IncludeNetwork = true - settings.IncludeCountry = true - settings.UploadResults = true - settings.SendCrashReports = true - - if changeDefaults == true { - var qs = []*survey.Question{ - { - Name: "IncludeIP", - Prompt: &survey.Confirm{Message: "Should we include your IP?"}, - }, - { - Name: "IncludeNetwork", - Prompt: &survey.Confirm{ - Message: "Can we include your network name?", - Default: true, - }, - }, - { - Name: "IncludeCountry", - Prompt: &survey.Confirm{ - Message: "Can we include your country name?", - Default: true, - }, - }, - { - Name: "UploadResults", - Prompt: &survey.Confirm{ - Message: "Can we upload your results?", - Default: true, - }, - }, - { - Name: "SendCrashReports", - Prompt: &survey.Confirm{ - Message: "Can we send crash reports to OONI?", - Default: true, - }, - }, - } - - err := survey.Ask(qs, &settings) - if err != nil { - log.WithError(err).Error("there was an error in parsing your responses") - return err - } - } - - config.Lock() - config.InformedConsent = true - config.Sharing.IncludeCountry = settings.IncludeCountry - config.Advanced.SendCrashReports = settings.SendCrashReports - config.Sharing.IncludeIP = settings.IncludeIP - config.Sharing.IncludeASN = settings.IncludeNetwork - config.Sharing.UploadResults = settings.UploadResults - config.Unlock() - - if err := config.Write(); err != nil { - log.WithError(err).Error("failed to write config file") - return err - } - return nil -} diff --git a/internal/output/output.go b/internal/output/output.go index f5ed02a..bd92717 100644 --- a/internal/output/output.go +++ b/internal/output/output.go @@ -8,7 +8,7 @@ import ( "github.com/apex/log" "github.com/ooni/probe-cli/internal/database" - "github.com/ooni/probe-cli/internal/util" + "github.com/ooni/probe-cli/utils" ) // MeasurementJSON prints the JSON of a measurement @@ -154,12 +154,12 @@ func SectionTitle(text string) { func Paragraph(text string) { const width = 80 - fmt.Println(util.WrapString(text, width)) + fmt.Println(utils.WrapString(text, width)) } func Bullet(text string) { const width = 80 - fmt.Printf("• %s\n", util.WrapString(text, width)) + fmt.Printf("• %s\n", utils.WrapString(text, width)) } func PressEnterToContinue(text string) { diff --git a/ooni.go b/ooni.go index 58d20eb..d799222 100644 --- a/ooni.go +++ b/ooni.go @@ -10,7 +10,6 @@ import ( "github.com/ooni/probe-cli/internal/database" "github.com/ooni/probe-cli/internal/enginex" "github.com/ooni/probe-cli/internal/legacy" - "github.com/ooni/probe-cli/internal/onboard" "github.com/ooni/probe-cli/utils" "github.com/ooni/probe-cli/version" engine "github.com/ooni/probe-engine" @@ -37,20 +36,6 @@ func (c *Context) MaybeLocationLookup() error { return c.Session.MaybeLookupLocation() } -// MaybeOnboarding will run the onboarding process only if the informed consent -// config option is set to false -func (c *Context) MaybeOnboarding() error { - if c.Config.InformedConsent == false { - if c.IsBatch == true { - return errors.New("cannot run onboarding in batch mode") - } - if err := onboard.Onboarding(c.Config); err != nil { - return errors.Wrap(err, "onboarding") - } - } - return nil -} - // Init the OONI manager func (c *Context) Init() error { var err error diff --git a/internal/shutil/shutil.go b/utils/shutil/shutil.go similarity index 100% rename from internal/shutil/shutil.go rename to utils/shutil/shutil.go diff --git a/internal/util/util_test.go b/utils/util_test.go similarity index 96% rename from internal/util/util_test.go rename to utils/util_test.go index 3a567c7..304be37 100644 --- a/internal/util/util_test.go +++ b/utils/util_test.go @@ -1,4 +1,4 @@ -package util +package utils import ( "testing" diff --git a/internal/util/util.go b/utils/utils.go similarity index 99% rename from internal/util/util.go rename to utils/utils.go index 7f48142..328e4da 100644 --- a/internal/util/util.go +++ b/utils/utils.go @@ -1,4 +1,4 @@ -package util +package utils import ( "bytes" From cda9a3ee4f2f85a934bafc8b400cb380e17bacea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arturo=20Filast=C3=B2?= Date: Mon, 2 Dec 2019 15:25:23 +0200 Subject: [PATCH 02/10] Add code coverage integration --- .travis.yml | 2 ++ build.sh | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 7bedfbf..427b40f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,3 +8,5 @@ env: script: - ./build.sh _travis-${TRAVIS_OS_NAME} - ./scripts/travis_test.sh +after_success: +- bash <(curl -s https://codecov.io/bash) diff --git a/build.sh b/build.sh index 9995cd3..c9c846e 100755 --- a/build.sh +++ b/build.sh @@ -51,7 +51,7 @@ elif [ "$1" = "_travis-linux" ]; then set -x $0 linux docker run -v `pwd`:/oonibuild -w /oonibuild -t oonibuild \ - go test -v -coverprofile=ooni.cov ./... + go test -v -coverprofile=coverage.txt -covermode=atomic ./... elif [ "$1" = "_travis-osx" ]; then set -x @@ -60,7 +60,7 @@ elif [ "$1" = "_travis-osx" ]; then brew upgrade brew install measurement-kit $0 macos - go test -v -coverprofile=ooni.cov ./... + go test -v -coverprofile=coverage.txt -covermode=atomic ./... elif [ "$1" = "help" ]; then echo "Usage: $0 linux | macos | release | windows" From a3685ae6b02d748caf6bbff8b97e065daf700812 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arturo=20Filast=C3=B2?= Date: Mon, 2 Dec 2019 15:26:12 +0200 Subject: [PATCH 03/10] Add coverage.txt to .gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index fde82b1..5ee9870 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ /dist /ooni.cov +/coverage.txt *.njson .DS_Store From 410b7514d7d1f198285725744b47f63f8afe8021 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arturo=20Filast=C3=B2?= Date: Mon, 2 Dec 2019 15:42:14 +0200 Subject: [PATCH 04/10] Use coveralls which is the same thing used in other repos --- .travis.yml | 6 ++++-- build.sh | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index 427b40f..d9312eb 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,8 +5,10 @@ services: - docker env: - OS_NAME: linux +before_script: +- go get golang.org/x/tools/cmd/cover +- go get github.com/mattn/goveralls script: - ./build.sh _travis-${TRAVIS_OS_NAME} +- $GOPATH/bin/goveralls -coverprofile=coverage.cov -service=travis-ci - ./scripts/travis_test.sh -after_success: -- bash <(curl -s https://codecov.io/bash) diff --git a/build.sh b/build.sh index c9c846e..76757d2 100755 --- a/build.sh +++ b/build.sh @@ -51,7 +51,7 @@ elif [ "$1" = "_travis-linux" ]; then set -x $0 linux docker run -v `pwd`:/oonibuild -w /oonibuild -t oonibuild \ - go test -v -coverprofile=coverage.txt -covermode=atomic ./... + go test -v -coverprofile=coverage.cov -covermode=atomic ./... elif [ "$1" = "_travis-osx" ]; then set -x @@ -60,7 +60,7 @@ elif [ "$1" = "_travis-osx" ]; then brew upgrade brew install measurement-kit $0 macos - go test -v -coverprofile=coverage.txt -covermode=atomic ./... + go test -v -coverprofile=coverage.cov -covermode=atomic ./... elif [ "$1" = "help" ]; then echo "Usage: $0 linux | macos | release | windows" From c99e719a45000c0c0a8f780fadf448af4045cd12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arturo=20Filast=C3=B2?= Date: Mon, 2 Dec 2019 16:01:37 +0200 Subject: [PATCH 05/10] Use code coverage commands from probe-engine --- build.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build.sh b/build.sh index 76757d2..0ab53f9 100755 --- a/build.sh +++ b/build.sh @@ -51,7 +51,7 @@ elif [ "$1" = "_travis-linux" ]; then set -x $0 linux docker run -v `pwd`:/oonibuild -w /oonibuild -t oonibuild \ - go test -v -coverprofile=coverage.cov -covermode=atomic ./... + go test -v -race -coverprofile=coverage.cov -coverpkg=./... ./... elif [ "$1" = "_travis-osx" ]; then set -x @@ -60,7 +60,7 @@ elif [ "$1" = "_travis-osx" ]; then brew upgrade brew install measurement-kit $0 macos - go test -v -coverprofile=coverage.cov -covermode=atomic ./... + go test -v -race -coverprofile=coverage.cov -coverpkg=./... ./... elif [ "$1" = "help" ]; then echo "Usage: $0 linux | macos | release | windows" From d6d2490d2c75884bf7d1f415fdedd90a6189a1fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arturo=20Filast=C3=B2?= Date: Mon, 2 Dec 2019 16:09:10 +0200 Subject: [PATCH 06/10] Disable -race as it fails on alpine --- build.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/build.sh b/build.sh index 0ab53f9..5e69e7d 100755 --- a/build.sh +++ b/build.sh @@ -50,8 +50,9 @@ elif [ "$1" = "release" ]; then elif [ "$1" = "_travis-linux" ]; then set -x $0 linux + # TODO -race does not work on alpine. See: https://travis-ci.org/ooni/probe-cli/builds/619631256#L962 docker run -v `pwd`:/oonibuild -w /oonibuild -t oonibuild \ - go test -v -race -coverprofile=coverage.cov -coverpkg=./... ./... + go test -v -coverprofile=coverage.cov -coverpkg=./... ./... elif [ "$1" = "_travis-osx" ]; then set -x From b20af107dc9e9b804cef95d7cb7ab9aa86115512 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arturo=20Filast=C3=B2?= Date: Mon, 2 Dec 2019 17:57:55 +0200 Subject: [PATCH 07/10] Refactor nettests into top tree --- nettests/{performance => }/dash.go | 6 +-- nettests/groups.go | 39 ++++++++++++++ nettests/groups/groups.go | 47 ----------------- .../http_header_field_manipulation.go | 6 +-- .../http_invalid_request_line.go | 6 +-- nettests/{performance => }/ndt.go | 5 +- nettests/nettests_test.go | 52 +++++++++++++++++++ nettests/{websites => }/web_connectivity.go | 7 ++- 8 files changed, 102 insertions(+), 66 deletions(-) rename nettests/{performance => }/dash.go (92%) create mode 100644 nettests/groups.go delete mode 100644 nettests/groups/groups.go rename nettests/{middlebox => }/http_header_field_manipulation.go (90%) rename nettests/{middlebox => }/http_invalid_request_line.go (88%) rename nettests/{performance => }/ndt.go (95%) create mode 100644 nettests/nettests_test.go rename nettests/{websites => }/web_connectivity.go (91%) diff --git a/nettests/performance/dash.go b/nettests/dash.go similarity index 92% rename from nettests/performance/dash.go rename to nettests/dash.go index d6096d6..9c6a81e 100644 --- a/nettests/performance/dash.go +++ b/nettests/dash.go @@ -1,9 +1,7 @@ -package performance +package nettests import ( "github.com/pkg/errors" - - "github.com/ooni/probe-cli/nettests" ) // Dash test implementation @@ -11,7 +9,7 @@ type Dash struct { } // Run starts the test -func (d Dash) Run(ctl *nettests.Controller) error { +func (d Dash) Run(ctl *Controller) error { builder, err := ctl.Ctx.Session.NewExperimentBuilder("dash") if err != nil { return err diff --git a/nettests/groups.go b/nettests/groups.go new file mode 100644 index 0000000..6ddc6ad --- /dev/null +++ b/nettests/groups.go @@ -0,0 +1,39 @@ +package nettests + +// NettestGroup base structure +type NettestGroup struct { + Label string + Nettests []Nettest +} + +// NettestGroups that can be run by the user +var NettestGroups = map[string]NettestGroup{ + "websites": NettestGroup{ + Label: "Websites", + Nettests: []Nettest{ + WebConnectivity{}, + }, + }, + "performance": NettestGroup{ + Label: "Performance", + Nettests: []Nettest{ + Dash{}, + NDT{}, + }, + }, + "middlebox": NettestGroup{ + Label: "Middleboxes", + Nettests: []Nettest{ + HTTPInvalidRequestLine{}, + HTTPHeaderFieldManipulation{}, + }, + }, + "im": NettestGroup{ + Label: "Instant Messaging", + Nettests: []Nettest{ + FacebookMessenger{}, + Telegram{}, + WhatsApp{}, + }, + }, +} diff --git a/nettests/groups/groups.go b/nettests/groups/groups.go deleted file mode 100644 index e209c55..0000000 --- a/nettests/groups/groups.go +++ /dev/null @@ -1,47 +0,0 @@ -package groups - -import ( - "github.com/ooni/probe-cli/nettests" - "github.com/ooni/probe-cli/nettests/im" - "github.com/ooni/probe-cli/nettests/middlebox" - "github.com/ooni/probe-cli/nettests/performance" - "github.com/ooni/probe-cli/nettests/websites" -) - -// NettestGroup base structure -type NettestGroup struct { - Label string - Nettests []nettests.Nettest -} - -// NettestGroups that can be run by the user -var NettestGroups = map[string]NettestGroup{ - "websites": NettestGroup{ - Label: "Websites", - Nettests: []nettests.Nettest{ - websites.WebConnectivity{}, - }, - }, - "performance": NettestGroup{ - Label: "Performance", - Nettests: []nettests.Nettest{ - performance.Dash{}, - performance.NDT{}, - }, - }, - "middlebox": NettestGroup{ - Label: "Middleboxes", - Nettests: []nettests.Nettest{ - middlebox.HTTPInvalidRequestLine{}, - middlebox.HTTPHeaderFieldManipulation{}, - }, - }, - "im": NettestGroup{ - Label: "Instant Messaging", - Nettests: []nettests.Nettest{ - im.FacebookMessenger{}, - im.Telegram{}, - im.WhatsApp{}, - }, - }, -} diff --git a/nettests/middlebox/http_header_field_manipulation.go b/nettests/http_header_field_manipulation.go similarity index 90% rename from nettests/middlebox/http_header_field_manipulation.go rename to nettests/http_header_field_manipulation.go index fe4d59e..f9d20bc 100644 --- a/nettests/middlebox/http_header_field_manipulation.go +++ b/nettests/http_header_field_manipulation.go @@ -1,9 +1,7 @@ -package middlebox +package nettests import ( "errors" - - "github.com/ooni/probe-cli/nettests" ) // HTTPHeaderFieldManipulation test implementation @@ -11,7 +9,7 @@ type HTTPHeaderFieldManipulation struct { } // Run starts the test -func (h HTTPHeaderFieldManipulation) Run(ctl *nettests.Controller) error { +func (h HTTPHeaderFieldManipulation) Run(ctl *Controller) error { builder, err := ctl.Ctx.Session.NewExperimentBuilder( "http_header_field_manipulation", ) diff --git a/nettests/middlebox/http_invalid_request_line.go b/nettests/http_invalid_request_line.go similarity index 88% rename from nettests/middlebox/http_invalid_request_line.go rename to nettests/http_invalid_request_line.go index 89ff081..4dcff14 100644 --- a/nettests/middlebox/http_invalid_request_line.go +++ b/nettests/http_invalid_request_line.go @@ -1,9 +1,7 @@ -package middlebox +package nettests import ( "errors" - - "github.com/ooni/probe-cli/nettests" ) // HTTPInvalidRequestLine test implementation @@ -11,7 +9,7 @@ type HTTPInvalidRequestLine struct { } // Run starts the test -func (h HTTPInvalidRequestLine) Run(ctl *nettests.Controller) error { +func (h HTTPInvalidRequestLine) Run(ctl *Controller) error { builder, err := ctl.Ctx.Session.NewExperimentBuilder( "http_invalid_request_line", ) diff --git a/nettests/performance/ndt.go b/nettests/ndt.go similarity index 95% rename from nettests/performance/ndt.go rename to nettests/ndt.go index cc6cb4d..9ca4c61 100644 --- a/nettests/performance/ndt.go +++ b/nettests/ndt.go @@ -1,7 +1,6 @@ -package performance +package nettests import ( - "github.com/ooni/probe-cli/nettests" "github.com/pkg/errors" ) @@ -10,7 +9,7 @@ type NDT struct { } // Run starts the test -func (n NDT) Run(ctl *nettests.Controller) error { +func (n NDT) Run(ctl *Controller) error { builder, err := ctl.Ctx.Session.NewExperimentBuilder("ndt") if err != nil { return err diff --git a/nettests/nettests_test.go b/nettests/nettests_test.go new file mode 100644 index 0000000..df10cb6 --- /dev/null +++ b/nettests/nettests_test.go @@ -0,0 +1,52 @@ +package nettests + +import ( + "io/ioutil" + "path" + "testing" + + ooni "github.com/ooni/probe-cli" + "github.com/ooni/probe-cli/internal/database" + "github.com/ooni/probe-cli/utils/shutil" +) + +func newTestingContext(t *testing.T) *ooni.Context { + homePath, err := ioutil.TempDir("", "ooniprobetests") + if err != nil { + t.Fatal(err) + } + configPath := path.Join(homePath, "config.json") + testingConfig := path.Join("..", "testdata", "testing-config.json") + shutil.Copy(testingConfig, configPath, false) + ctx := ooni.NewContext(configPath, homePath) + err = ctx.Init() + if err != nil { + t.Fatal(err) + } + return ctx +} + +func TestCreateContext(t *testing.T) { + newTestingContext(t) +} + +func TestRun(t *testing.T) { + ctx := newTestingContext(t) + network, err := database.CreateNetwork(ctx.DB, ctx.Session) + if err != nil { + t.Fatal(err) + } + res, err := database.CreateResult(ctx.DB, ctx.Home, tg, network.ID) + if err != nil { + t.Fatal(err) + } + builder, err := ctl.Ctx.Session.NewExperimentBuilder( + "telegram", + ) + if err != nil { + t.Fatal(err) + } + ctl := NewController(nt, ctx, res) + ctl.Run(builder, []string{""}) + nt.Run(ctl) +} diff --git a/nettests/websites/web_connectivity.go b/nettests/web_connectivity.go similarity index 91% rename from nettests/websites/web_connectivity.go rename to nettests/web_connectivity.go index 63a457a..2cf53d4 100644 --- a/nettests/websites/web_connectivity.go +++ b/nettests/web_connectivity.go @@ -1,12 +1,11 @@ -package websites +package nettests import ( "github.com/apex/log" "github.com/ooni/probe-cli/internal/database" - "github.com/ooni/probe-cli/nettests" ) -func lookupURLs(ctl *nettests.Controller, limit int64) ([]string, map[int64]int64, error) { +func lookupURLs(ctl *Controller, limit int64) ([]string, map[int64]int64, error) { var urls []string urlIDMap := make(map[int64]int64) config := ctl.Ctx.Session.NewTestListsConfig() @@ -37,7 +36,7 @@ type WebConnectivity struct { } // Run starts the test -func (n WebConnectivity) Run(ctl *nettests.Controller) error { +func (n WebConnectivity) Run(ctl *Controller) error { urls, urlIDMap, err := lookupURLs(ctl, ctl.Ctx.Config.Nettests.WebsitesURLLimit) if err != nil { return err From ccdff3ba247de2ac4b6178fe21aec20e0581d2af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arturo=20Filast=C3=B2?= Date: Mon, 2 Dec 2019 17:59:37 +0200 Subject: [PATCH 08/10] Move IM tests into top level tree too --- nettests/{im => }/facebook_messenger.go | 8 ++------ nettests/nettests_test.go | 10 ++-------- nettests/{im => }/telegram.go | 8 ++------ nettests/{im => }/whatsapp.go | 8 ++------ 4 files changed, 8 insertions(+), 26 deletions(-) rename nettests/{im => }/facebook_messenger.go (90%) rename nettests/{im => }/telegram.go (92%) rename nettests/{im => }/whatsapp.go (93%) diff --git a/nettests/im/facebook_messenger.go b/nettests/facebook_messenger.go similarity index 90% rename from nettests/im/facebook_messenger.go rename to nettests/facebook_messenger.go index 02c8b84..7e8067c 100644 --- a/nettests/im/facebook_messenger.go +++ b/nettests/facebook_messenger.go @@ -1,15 +1,11 @@ -package im - -import ( - "github.com/ooni/probe-cli/nettests" -) +package nettests // FacebookMessenger test implementation type FacebookMessenger struct { } // Run starts the test -func (h FacebookMessenger) Run(ctl *nettests.Controller) error { +func (h FacebookMessenger) Run(ctl *Controller) error { builder, err := ctl.Ctx.Session.NewExperimentBuilder( "facebook_messenger", ) diff --git a/nettests/nettests_test.go b/nettests/nettests_test.go index df10cb6..364bf6b 100644 --- a/nettests/nettests_test.go +++ b/nettests/nettests_test.go @@ -36,17 +36,11 @@ func TestRun(t *testing.T) { if err != nil { t.Fatal(err) } - res, err := database.CreateResult(ctx.DB, ctx.Home, tg, network.ID) - if err != nil { - t.Fatal(err) - } - builder, err := ctl.Ctx.Session.NewExperimentBuilder( - "telegram", - ) + res, err := database.CreateResult(ctx.DB, ctx.Home, "im", network.ID) if err != nil { t.Fatal(err) } + nt := Telegram{} ctl := NewController(nt, ctx, res) - ctl.Run(builder, []string{""}) nt.Run(ctl) } diff --git a/nettests/im/telegram.go b/nettests/telegram.go similarity index 92% rename from nettests/im/telegram.go rename to nettests/telegram.go index ed91e00..3601066 100644 --- a/nettests/im/telegram.go +++ b/nettests/telegram.go @@ -1,15 +1,11 @@ -package im - -import ( - "github.com/ooni/probe-cli/nettests" -) +package nettests // Telegram test implementation type Telegram struct { } // Run starts the test -func (h Telegram) Run(ctl *nettests.Controller) error { +func (h Telegram) Run(ctl *Controller) error { builder, err := ctl.Ctx.Session.NewExperimentBuilder( "telegram", ) diff --git a/nettests/im/whatsapp.go b/nettests/whatsapp.go similarity index 93% rename from nettests/im/whatsapp.go rename to nettests/whatsapp.go index c5ee86f..57a1aca 100644 --- a/nettests/im/whatsapp.go +++ b/nettests/whatsapp.go @@ -1,15 +1,11 @@ -package im - -import ( - "github.com/ooni/probe-cli/nettests" -) +package nettests // WhatsApp test implementation type WhatsApp struct { } // Run starts the test -func (h WhatsApp) Run(ctl *nettests.Controller) error { +func (h WhatsApp) Run(ctl *Controller) error { builder, err := ctl.Ctx.Session.NewExperimentBuilder( "whatsapp", ) From b7deb6c88fb8efb886c9f2c77ed52d0be8b95795 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arturo=20Filast=C3=B2?= Date: Mon, 2 Dec 2019 18:00:12 +0200 Subject: [PATCH 09/10] Use a faster nettest for testing --- nettests/nettests_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nettests/nettests_test.go b/nettests/nettests_test.go index 364bf6b..3358bec 100644 --- a/nettests/nettests_test.go +++ b/nettests/nettests_test.go @@ -36,11 +36,11 @@ func TestRun(t *testing.T) { if err != nil { t.Fatal(err) } - res, err := database.CreateResult(ctx.DB, ctx.Home, "im", network.ID) + res, err := database.CreateResult(ctx.DB, ctx.Home, "middlebox", network.ID) if err != nil { t.Fatal(err) } - nt := Telegram{} + nt := HTTPInvalidRequestLine{} ctl := NewController(nt, ctx, res) nt.Run(ctl) } From 274e533b2e0df987e2c71080eb0b93069d5aa2b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arturo=20Filast=C3=B2?= Date: Mon, 2 Dec 2019 18:05:02 +0200 Subject: [PATCH 10/10] Fix import for groups --- internal/cli/run/run.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/internal/cli/run/run.go b/internal/cli/run/run.go index 8090180..066c205 100644 --- a/internal/cli/run/run.go +++ b/internal/cli/run/run.go @@ -11,11 +11,10 @@ import ( "github.com/ooni/probe-cli/internal/cli/root" "github.com/ooni/probe-cli/internal/database" "github.com/ooni/probe-cli/nettests" - "github.com/ooni/probe-cli/nettests/groups" ) func runNettestGroup(tg string, ctx *ooni.Context, network *database.Network) error { - group, ok := groups.NettestGroups[tg] + group, ok := nettests.NettestGroups[tg] if !ok { log.Errorf("No test group named %s", tg) return errors.New("invalid test group name") @@ -51,7 +50,7 @@ func init() { var ctx *ooni.Context var network *database.Network - for name := range groups.NettestGroups { + for name := range nettests.NettestGroups { nettestGroupNamesBlue = append(nettestGroupNamesBlue, color.BlueString(name)) } @@ -130,7 +129,7 @@ func init() { allCmd := cmd.Command("all", "").Default() allCmd.Action(func(_ *kingpin.ParseContext) error { log.Infof("Running %s tests", color.BlueString("all")) - for tg := range groups.NettestGroups { + for tg := range nettests.NettestGroups { if err := runNettestGroup(tg, ctx, network); err != nil { log.WithError(err).Errorf("failed to run %s", tg) }