Repository: uber/prototool
Pull request #291 (2019-01-11 17:28:36)

Fail break check if a stable package depends on a beta package unless --allow-beta-deps is set
Re: #265

codecov-io: # [Codecov](https://codecov.io/gh/uber/prototool/pull/291?src=pr&el=h1) Report
> Merging [#291](https://codecov.io/gh/uber/prototool/pull/291?src=pr&el=desc) into [dev](https://codecov.io/gh/uber/prototool/commit/fb93ac4a25a981be9db8a727e33eb9d028d80460?src=pr&el=desc) will **increase** coverage by `0.09%`.
> The diff coverage is `68.96%`.

[![Impacted file tree graph](https://codecov.io/gh/uber/prototool/pull/291/graphs/tree.svg?width=650&token=A9V0DApqZg&height=150&src=pr)](https://codecov.io/gh/uber/prototool/pull/291?src=pr&el=tree)

```diff
@@            Coverage Diff            @@
##              dev    #291      +/-   ##
=========================================
+ Coverage   72.51%   72.6%   +0.09%     
=========================================
  Files         124     125       +1     
  Lines        5639    5658      +19     
=========================================
+ Hits         4089    4108      +19     
- Misses       1163    1164       +1     
+ Partials      387     386       -1
```


| [Impacted Files](https://codecov.io/gh/uber/prototool/pull/291?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [internal/exec/exec.go](https://codecov.io/gh/uber/prototool/pull/291/diff?src=pr&el=tree#diff-aW50ZXJuYWwvZXhlYy9leGVjLmdv) | `57.14% <ø> (ø)` | :arrow_up: |
| [internal/exec/runner.go](https://codecov.io/gh/uber/prototool/pull/291/diff?src=pr&el=tree#diff-aW50ZXJuYWwvZXhlYy9ydW5uZXIuZ28=) | `46.65% <0%> (-0.37%)` | :arrow_down: |
| [internal/breaking/breaking.go](https://codecov.io/gh/uber/prototool/pull/291/diff?src=pr&el=tree#diff-aW50ZXJuYWwvYnJlYWtpbmcvYnJlYWtpbmcuZ28=) | `72.72% <100%> (+10.22%)` | :arrow_up: |
| [internal/breaking/check\_packages\_no\_beta\_deps.go](https://codecov.io/gh/uber/prototool/pull/291/diff?src=pr&el=tree#diff-aW50ZXJuYWwvYnJlYWtpbmcvY2hlY2tfcGFja2FnZXNfbm9fYmV0YV9kZXBzLmdv) | `100% <100%> (ø)` | |
| [internal/cmd/flags.go](https://codecov.io/gh/uber/prototool/pull/291/diff?src=pr&el=tree#diff-aW50ZXJuYWwvY21kL2ZsYWdzLmdv) | `100% <100%> (ø)` | :arrow_up: |
| [internal/breaking/runner.go](https://codecov.io/gh/uber/prototool/pull/291/diff?src=pr&el=tree#diff-aW50ZXJuYWwvYnJlYWtpbmcvcnVubmVyLmdv) | `79.31% <100%> (ø)` | :arrow_up: |
| [internal/cmd/templates.go](https://codecov.io/gh/uber/prototool/pull/291/diff?src=pr&el=tree#diff-aW50ZXJuYWwvY21kL3RlbXBsYXRlcy5nbw==) | `82.11% <50%> (+0.07%)` | :arrow_up: |
| [internal/extract/extract.go](https://codecov.io/gh/uber/prototool/pull/291/diff?src=pr&el=tree#diff-aW50ZXJuYWwvZXh0cmFjdC9leHRyYWN0Lmdv) | `89.69% <0%> (+3.03%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/uber/prototool/pull/291?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/uber/prototool/pull/291?src=pr&el=footer). Last update [fb93ac4...5f04cbd](https://codecov.io/gh/uber/prototool/pull/291?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/internal/breaking/breaking.go b/internal/breaking/breaking.go
index dbad789..67bd3ac 100644
--- a/internal/breaking/breaking.go
+++ b/internal/breaking/breaking.go
@@ -118,6 +118,14 @@ var (
 			Check:   checkServiceMethodsSameServerStreaming,
 		},
 	}
+
+	// PackagesNoBetaDepsChecker is a special checker that verifies no stable packages
+	// import beta packages.
+	PackagesNoBetaDepsChecker = Checker{
+		ID:      "PACKAGES_NO_BETA_DEPS",
+		Purpose: "Checks that stable packages do not have beta dependencies.",
+		Check:   checkPackagesNoBetaDeps,
+	}
 )
 
 // Checker checks compatibility.
@@ -162,6 +170,18 @@ func RunnerWithIncludeBeta() RunnerOption {
 	}
 }
 
+// RunnerWithAllowBetaDeps returns a RunnerOption that allows beta dependencies
+// in stable packages.
+//
+// Setting RunnerWithIncludeBeta will cause this to be true.
+//
+// The default is to not allow beta dependencies in stable packages.
+func RunnerWithAllowBetaDeps() RunnerOption {
+	return func(runner *runner) {
+		runner.allowBetaDeps = true
+	}
+}
+
 // NewRunner returns a new Runner.
 func NewRunner(options ...RunnerOption) Runner {
 	return newRunner(options...)
diff --git a/internal/breaking/check_packages_no_beta_deps.go b/internal/breaking/check_packages_no_beta_deps.go
new file mode 100644
index 0000000..27fa3e4
--- /dev/null
+++ b/internal/breaking/check_packages_no_beta_deps.go
@@ -0,0 +1,44 @@
+// Copyright (c) 2019 Uber Technologies, Inc.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+
+package breaking
+
+import (
+	"github.com/uber/prototool/internal/extract"
+	"github.com/uber/prototool/internal/protostrs"
+	"github.com/uber/prototool/internal/text"
+)
+
+func checkPackagesNoBetaDeps(addFailure func(*text.Failure), from *extract.PackageSet, to *extract.PackageSet) error {
+	// this check is only run if WithIncludeBeta is not set
+	// so this map will only contain stable packages
+	for _, toPackage := range to.PackageNameToPackage() {
+		for _, depPackageName := range toPackage.ProtoMessage().DependencyNames {
+			if _, betaVersion, ok := protostrs.MajorBetaVersion(depPackageName); ok && betaVersion > 0 {
+				addFailure(newPackagesNoBetaDepsFailure(toPackage.FullyQualifiedName(), depPackageName))
+			}
+		}
+	}
+	return nil
+}
+
+func newPackagesNoBetaDepsFailure(packageName string, depPackageName string) *text.Failure {
+	return newTextFailuref(`Package %q depends on beta package %q which is not allowed.`, packageName, depPackageName)
+}
diff --git a/internal/breaking/runner.go b/internal/breaking/runner.go
index 6322ef6..b858ca1 100644
--- a/internal/breaking/runner.go
+++ b/internal/breaking/runner.go
@@ -27,9 +27,10 @@ import (
 )
 
 type runner struct {
-	logger      *zap.Logger
-	includeBeta bool
-	checkers    []Checker
+	logger        *zap.Logger
+	includeBeta   bool
+	allowBetaDeps bool
+	checkers      []Checker
 }
 
 func newRunner(options ...RunnerOption) *runner {
@@ -55,22 +56,24 @@ func (r *runner) Run(from *extract.PackageSet, to *extract.PackageSet) ([]*text.
 			return nil, err
 		}
 	}
+	checkers := r.checkers
+	// if includeBeta, do not do the check
+	// else if not including beta, unless allow beta deps, do not do the check
+	if !r.includeBeta && !r.allowBetaDeps {
+		checkers = append(checkers, PackagesNoBetaDepsChecker)
+	}
 	var failures []*text.Failure
-	for _, checker := range r.checkers {
-		var checkerFailures []*text.Failure
+	for _, checker := range checkers {
 		if err := checker.Check(
 			func(failure *text.Failure) {
-				checkerFailures = append(checkerFailures, failure)
+				failure.LintID = checker.ID
+				failures = append(failures, failure)
 			},
 			from,
 			to,
 		); err != nil {
 			return nil, err
 		}
-		for _, checkerFailure := range checkerFailures {
-			checkerFailure.LintID = checker.ID
-			failures = append(failures, checkerFailure)
-		}
 	}
 	return failures, nil
 }
diff --git a/internal/cmd/flags.go b/internal/cmd/flags.go
index c16e85a..1ecb7cc 100644
--- a/internal/cmd/flags.go
+++ b/internal/cmd/flags.go
@@ -25,6 +25,7 @@ import (
 )
 
 type flags struct {
+	allowBetaDeps     bool
 	address           string
 	cachePath         string
 	callTimeout       string
@@ -61,6 +62,10 @@ type flags struct {
 	uncomment         bool
 }
 
+func (f *flags) bindAllowBetaDeps(flagSet *pflag.FlagSet) {
+	flagSet.BoolVar(&f.allowBetaDeps, "allow-beta-deps", false, "Allow stable packages to depend on beta packages. This is implicitly set if --include-beta is set.")
+}
+
 func (f *flags) bindAddress(flagSet *pflag.FlagSet) {
 	flagSet.StringVar(&f.address, "address", "", "The GRPC endpoint to connect to. This is required.")
 }
diff --git a/internal/cmd/templates.go b/internal/cmd/templates.go
index ce47975..8807051 100644
--- a/internal/cmd/templates.go
+++ b/internal/cmd/templates.go
@@ -65,9 +65,10 @@ var (
 The input directory must be relative.`,
 		Args: cobra.MaximumNArgs(1),
 		Run: func(runner exec.Runner, args []string, flags *flags) error {
-			return runner.BreakCheck(args, flags.gitBranch, flags.gitTag, flags.includeBeta)
+			return runner.BreakCheck(args, flags.gitBranch, flags.gitTag, flags.includeBeta, flags.allowBetaDeps)
 		},
 		BindFlags: func(flagSet *pflag.FlagSet, flags *flags) {
+			flags.bindAllowBetaDeps(flagSet)
 			flags.bindCachePath(flagSet)
 			flags.bindConfigData(flagSet)
 			flags.bindGitBranch(flagSet)
diff --git a/internal/exec/exec.go b/internal/exec/exec.go
index 149d094..56ac477 100644
--- a/internal/exec/exec.go
+++ b/internal/exec/exec.go
@@ -60,7 +60,7 @@ type Runner interface {
 	InspectPackages(args []string) error
 	InspectPackageDeps(args []string, name string) error
 	InspectPackageImporters(args []string, name string) error
-	BreakCheck(args []string, gitBranch string, gitTag string, includeBeta bool) error
+	BreakCheck(args []string, gitBranch string, gitTag string, includeBeta bool, allowBetaDeps bool) error
 }
 
 // RunnerOption is an option for a new Runner.
diff --git a/internal/exec/runner.go b/internal/exec/runner.go
index 174525b..2bbe0fe 100644
--- a/internal/exec/runner.go
+++ b/internal/exec/runner.go
@@ -585,7 +585,7 @@ func (r *runner) InspectPackageImporters(args []string, name string) error {
 	return r.printPackageNames(pkg.ImporterNameToImporter())
 }
 
-func (r *runner) BreakCheck(args []string, gitBranch string, gitTag string, includeBeta bool) error {
+func (r *runner) BreakCheck(args []string, gitBranch string, gitTag string, includeBeta bool, allowBetaDeps bool) error {
 	if moreThanOneSet(gitBranch != "", gitTag != "") {
 		return newExitErrorf(255, "can only set one of git-branch, git-tag")
 	}
@@ -635,7 +635,7 @@ func (r *runner) BreakCheck(args []string, gitBranch string, gitTag string, incl
 		return err
 	}
 
-	failures, err := r.newBreakingRunner(includeBeta).Run(fromPackageSet, toPackageSet)
+	failures, err := r.newBreakingRunner(includeBeta, allowBetaDeps).Run(fromPackageSet, toPackageSet)
 	if err != nil {
 		return err
 	}
@@ -705,7 +705,7 @@ func (r *runner) getPackageSetForRelDirPath(relDirPath string) (*extract.Package
 	return r.getPackageSet([]string{dirPath})
 }
 
-func (r *runner) newBreakingRunner(includeBeta bool) breaking.Runner {
+func (r *runner) newBreakingRunner(includeBeta bool, allowBetaDeps bool) breaking.Runner {
 	runnerOptions := []breaking.RunnerOption{
 		breaking.RunnerWithLogger(r.logger),
 	}
@@ -715,6 +715,12 @@ func (r *runner) newBreakingRunner(includeBeta bool) breaking.Runner {
 			breaking.RunnerWithIncludeBeta(),
 		)
 	}
+	if allowBetaDeps {
+		runnerOptions = append(
+			runnerOptions,
+			breaking.RunnerWithAllowBetaDeps(),
+		)
+	}
 	return breaking.NewRunner(runnerOptions...)
 }
 
diff --git a/internal/lint/check_message_fields_not_floats.go b/internal/lint/check_message_fields_not_floats.go
index 787ae5f..0927aaf 100644
--- a/internal/lint/check_message_fields_not_floats.go
+++ b/internal/lint/check_message_fields_not_floats.go
@@ -31,7 +31,7 @@ const messageFieldsNotFloatsSuppressableAnnotation = "floats"
 
 var messageFieldsNotFloatsLinter = NewLinter(
 	"MESSAGE_FIELDS_NOT_FLOATS",
-	fmt.Sprintf(`Suppressable with "@suppresswarnings %s". Verifies that all message fields are not floats or doubles.`, messageFieldsNotFloatsSuppressableAnnotation),
+	fmt.Sprintf(`Suppressable with "@suppresswarnings %s". Verifies that all message fields are not floats.`, messageFieldsNotFloatsSuppressableAnnotation),
 	checkMessageFieldsNotFloats,
 )
 
@@ -69,10 +69,10 @@ func (v messageFieldsNotFloatsVisitor) VisitMapField(field *proto.MapField) {
 
 func (v messageFieldsNotFloatsVisitor) checkNotFloat(field *proto.Field) {
 	switch field.Type {
-	case "double", "float":
+	case "float":
 		if isSuppressed(field.Comment, messageFieldsNotFloatsSuppressableAnnotation) {
 			return
 		}
-		v.AddFailuref(field.Position, `Field %q is a float and floating point types should generally not be used, consider using an int64 while representing your value in micros or nanos. This can be suppressed by adding "@suppresswarnings %s" to the field comment.`, field.Name, messageFieldsNotFloatsSuppressableAnnotation)
+		v.AddFailuref(field.Position, `Field %q is a float and floats should generally not be used, consider using a double, or an int64 while representing your value in micros or nanos. This can be suppressed by adding "@suppresswarnings %s" to the field comment.`, field.Name, messageFieldsNotFloatsSuppressableAnnotation)
 	}
 }


Test patch:
diff --git a/internal/breaking/breaking_test.go b/internal/breaking/breaking_test.go
index 0310d60..879613d 100644
--- a/internal/breaking/breaking_test.go
+++ b/internal/breaking/breaking_test.go
@@ -35,7 +35,9 @@ func TestRunOne(t *testing.T) {
 		t,
 		"one",
 		false,
+		false,
 		newPackagesNotDeletedFailure("bar.v1"),
+		newPackagesNoBetaDepsFailure("foo.v1", "bar.v1beta1"),
 		newMessagesNotDeletedFailure("foo.v1.One.NestedOne.NestedNestedTwo"),
 		newMessagesNotDeletedFailure("foo.v1.One.NestedTwo"),
 		newMessagesNotDeletedFailure("foo.v1.Two"),
@@ -111,6 +113,7 @@ func TestRunOneIncludeBeta(t *testing.T) {
 		t,
 		"one",
 		true,
+		false,
 		newPackagesNotDeletedFailure("bar.v1"),
 		newPackagesNotDeletedFailure("foo.v1beta2"),
 		newMessagesNotDeletedFailure("foo.v1.One.NestedOne.NestedNestedTwo"),
@@ -250,13 +253,93 @@ func TestRunOneIncludeBeta(t *testing.T) {
 	)
 }
 
-func testRun(t *testing.T, subDirPath string, includeBeta bool, expectedFailures ...*text.Failure) {
+func TestRunOneAllowBetaDeps(t *testing.T) {
+	testRun(
+		t,
+		"one",
+		false,
+		true,
+		newPackagesNotDeletedFailure("bar.v1"),
+		newMessagesNotDeletedFailure("foo.v1.One.NestedOne.NestedNestedTwo"),
+		newMessagesNotDeletedFailure("foo.v1.One.NestedTwo"),
+		newMessagesNotDeletedFailure("foo.v1.Two"),
+		newMessageFieldsNotDeletedFailure("foo.v1.Three", 2),
+		newMessageFieldsNotDeletedFailure("foo.v1.Three.NestedThree", 2),
+		newMessageFieldsNotDeletedFailure("foo.v1.Three.NestedThree.NestedNestedThree", 2),
+		newMessageFieldsSameTypeFailure("foo.v1.Four", 1, "int64", "int32"),
+		newMessageFieldsSameTypeFailure("foo.v1.Four.NestedFour", 1, "int64", "int32"),
+		newMessageFieldsSameTypeFailure("foo.v1.Four.NestedFour.NestedNestedFour", 1, "int64", "int32"),
+		newMessageFieldsSameTypeFailure("foo.v1.Four", 2, "string", "bytes"),
+		newMessageFieldsSameTypeFailure("foo.v1.Four.NestedFour", 2, "string", "bytes"),
+		newMessageFieldsSameTypeFailure("foo.v1.Four.NestedFour.NestedNestedFour", 2, "string", "bytes"),
+		newMessageFieldsSameTypeFailure("foo.v1.Four", 3, "foo.v1.Four.NestedFour", "foo.v1.One"),
+		newMessageFieldsSameTypeFailure("foo.v1.Four.NestedFour", 3, "foo.v1.Four.NestedFour.NestedNestedFour", "foo.v1.One"),
+		newMessageFieldsSameTypeFailure("foo.v1.Four", 4, "foo.v1.EnumOne", "foo.v1.EnumThree"),
+		newMessageFieldsSameTypeFailure("foo.v1.Four.NestedFour", 4, "foo.v1.EnumOne", "foo.v1.EnumThree"),
+		newMessageFieldsSameTypeFailure("foo.v1.Four.NestedFour.NestedNestedFour", 4, "foo.v1.EnumOne", "foo.v1.EnumThree"),
+		newMessageFieldsSameTypeFailure("foo.v1.Four", 5, "enum", "double"),
+		newMessageFieldsSameTypeFailure("foo.v1.Four.NestedFour", 5, "enum", "double"),
+		newMessageFieldsSameTypeFailure("foo.v1.Four.NestedFour.NestedNestedFour", 5, "enum", "double"),
+		newMessageFieldsSameTypeFailure("foo.v1.Four", 6, "int64", "int32"),
+		newMessageFieldsSameTypeFailure("foo.v1.Four.NestedFour", 6, "int64", "int32"),
+		newMessageFieldsSameTypeFailure("foo.v1.Four.NestedFour.NestedNestedFour", 6, "int64", "int32"),
+		newMessageFieldsSameTypeFailure("foo.v1.Four.SevenEntry", 1, "int64", "int32"),
+		newMessageFieldsSameTypeFailure("foo.v1.Four.NestedFour.SevenEntry", 1, "int64", "int32"),
+		newMessageFieldsSameTypeFailure("foo.v1.Four.NestedFour.NestedNestedFour.SevenEntry", 1, "int64", "int32"),
+		newMessagesNotDeletedFailure("foo.v1.Five.FourEntry"),
+		newMessageFieldsSameLabelFailure("foo.v1.Five", 1, "optional", "repeated"),
+		newMessageFieldsSameLabelFailure("foo.v1.Five", 2, "optional", "repeated"),
+		newMessageFieldsSameTypeFailure("foo.v1.Five", 2, "string", "message"),
+		newMessageFieldsSameLabelFailure("foo.v1.Five", 3, "repeated", "optional"),
+		newMessageFieldsSameLabelFailure("foo.v1.Five", 4, "repeated", "optional"),
+		newMessageFieldsSameTypeFailure("foo.v1.Five", 4, "message", "int64"),
+		newEnumsNotDeletedFailure("foo.v1.EnumTwo"),
+		newEnumsNotDeletedFailure("foo.v1.Six.Foo"),
+		newEnumsNotDeletedFailure("foo.v1.Six.NestedSix.Foo"),
+		newEnumsNotDeletedFailure("foo.v1.Six.NestedSix.NestedNestedSix.Foo"),
+		newMessagesNotDeletedFailure("foo.v1.Six.NestedSix.NestedNestedSixDelete"),
+		newEnumValuesNotDeletedFailure("foo.v1.EnumSeven", 2),
+		newEnumValuesNotDeletedFailure("foo.v1.Seven.EnumSeven", 2),
+		newEnumValuesNotDeletedFailure("foo.v1.Seven.NestedSeven.EnumSeven", 2),
+		newEnumValuesNotDeletedFailure("foo.v1.Seven.NestedSeven.NestedNestedSeven.EnumSeven", 2),
+		newEnumValuesSameNameFailure("foo.v1.EnumSeven", 1, "ENUM_SEVEN_ONE", "ENUM_SEVEN_TWO"),
+		newEnumValuesSameNameFailure("foo.v1.Seven.EnumSeven", 1, "ENUM_SEVEN_ONE", "ENUM_SEVEN_TWO"),
+		newEnumValuesSameNameFailure("foo.v1.Seven.NestedSeven.EnumSeven", 1, "ENUM_SEVEN_ONE", "ENUM_SEVEN_TWO"),
+		newEnumValuesSameNameFailure("foo.v1.Seven.NestedSeven.NestedNestedSeven.EnumSeven", 1, "ENUM_SEVEN_ONE", "ENUM_SEVEN_TWO"),
+		newMessageOneofsNotDeletedFailure("foo.v1.Eight", "test"),
+		newMessageOneofsNotDeletedFailure("foo.v1.Eight.NestedEight", "test"),
+		newMessageOneofsNotDeletedFailure("foo.v1.Eight.NestedEight.NestedNestedEight", "test"),
+		newServicesNotDeletedFailure("foo.v1.TwoAPI"),
+		newServiceMethodsNotDeletedFailure("foo.v1.OneAPI", "OneTwo"),
+		newMessagesNotDeletedFailure("foo.v1.OneTwoRequest"),
+		newMessagesNotDeletedFailure("foo.v1.OneTwoResponse"),
+		newMessageFieldsSameNameFailure("foo.v1.Nine", 1, "one", "two"),
+		newMessageFieldsSameNameFailure("foo.v1.Nine.NestedNine", 1, "one", "two"),
+		newMessageFieldsSameNameFailure("foo.v1.Nine.NestedNine.NestedNestedNine", 1, "one", "two"),
+		newMessageOneofsFieldsNotRemovedFailure("foo.v1.Ten", "test", 3),
+		newMessageOneofsFieldsNotRemovedFailure("foo.v1.Ten.NestedTen", "test", 3),
+		newMessageOneofsFieldsNotRemovedFailure("foo.v1.Ten.NestedTen.NestedNestedTen", "test", 3),
+		newServiceMethodsSameRequestTypeFailure("foo.v1.ThreeAPI", "ThreeOne", "foo.v1.ThreeOneRequest", "foo.v1.OneOneRequest"),
+		newServiceMethodsSameResponseTypeFailure("foo.v1.ThreeAPI", "ThreeOne", "foo.v1.ThreeOneResponse", "foo.v1.OneOneResponse"),
+		newMessagesNotDeletedFailure("foo.v1.ThreeOneRequest"),
+		newMessagesNotDeletedFailure("foo.v1.ThreeOneResponse"),
+		newServiceMethodsSameClientStreamingFailure("foo.v1.ThreeAPI", "ThreeTwo", true),
+		newServiceMethodsSameClientStreamingFailure("foo.v1.ThreeAPI", "ThreeThree", false),
+		newServiceMethodsSameServerStreamingFailure("foo.v1.ThreeAPI", "ThreeFour", true),
+		newServiceMethodsSameServerStreamingFailure("foo.v1.ThreeAPI", "ThreeFive", false),
+	)
+}
+
+func testRun(t *testing.T, subDirPath string, includeBeta bool, allowBetaDeps bool, expectedFailures ...*text.Failure) {
 	fromPackageSet, toPackageSet, err := getPackageSets(subDirPath)
 	require.NoError(t, err)
 	runner := NewRunner()
 	if includeBeta {
 		runner = NewRunner(RunnerWithIncludeBeta())
 	}
+	if allowBetaDeps {
+		runner = NewRunner(RunnerWithAllowBetaDeps())
+	}
 	failures, err := runner.Run(fromPackageSet, toPackageSet)
 	require.NoError(t, err)
 	for _, failure := range failures {
diff --git a/internal/breaking/testdata/one/from/bar/v1beta1/bar.proto b/internal/breaking/testdata/one/from/bar/v1beta1/bar.proto
new file mode 100644
index 0000000..2a34b77
--- /dev/null
+++ b/internal/breaking/testdata/one/from/bar/v1beta1/bar.proto
@@ -0,0 +1,13 @@
+syntax = "proto3";
+
+package bar.v1beta1;
+
+option csharp_namespace = "Bar.V1Beta1";
+option go_package = "barv1beta1";
+option java_multiple_files = true;
+option java_outer_classname = "BarProto";
+option java_package = "com.bar.v1beta1";
+option objc_class_prefix = "BXX";
+option php_namespace = "Bar\\V1Beta1";
+
+message One {}
diff --git a/internal/breaking/testdata/one/from/bar/v2/bar.proto b/internal/breaking/testdata/one/from/bar/v2/bar.proto
new file mode 100644
index 0000000..9dc3dd7
--- /dev/null
+++ b/internal/breaking/testdata/one/from/bar/v2/bar.proto
@@ -0,0 +1,13 @@
+syntax = "proto3";
+
+package bar.v2;
+
+option csharp_namespace = "Bar.V2";
+option go_package = "barv2";
+option java_multiple_files = true;
+option java_outer_classname = "BarProto";
+option java_package = "com.bar.v2";
+option objc_class_prefix = "BXX";
+option php_namespace = "Bar\\V2";
+
+message One {}
diff --git a/internal/breaking/testdata/one/from/foo/v1beta1/foo.proto b/internal/breaking/testdata/one/from/foo/v1beta1/foo.proto
index ddb2248..2dfd0aa 100644
--- a/internal/breaking/testdata/one/from/foo/v1beta1/foo.proto
+++ b/internal/breaking/testdata/one/from/foo/v1beta1/foo.proto
@@ -10,6 +10,14 @@ option java_package = "com.foo.v1beta1";
 option objc_class_prefix = "FXX";
 option php_namespace = "Foo\\V1Beta1";
 
+import "bar/v1beta1/bar.proto";
+import "bar/v2/bar.proto";
+
+message Deps {
+  bar.v1beta1.One v1beta1_one = 2;
+  bar.v2.One v2_one = 1;
+}
+
 enum EnumOne {
   ENUM_ONE_INVALID = 0;
   ENUM_ONE_ONE = 1;
diff --git a/internal/breaking/testdata/one/from/foo/v1beta2/foo.proto b/internal/breaking/testdata/one/from/foo/v1beta2/foo.proto
index af309fa..141ed32 100644
--- a/internal/breaking/testdata/one/from/foo/v1beta2/foo.proto
+++ b/internal/breaking/testdata/one/from/foo/v1beta2/foo.proto
@@ -10,6 +10,14 @@ option java_package = "com.foo.v1beta2";
 option objc_class_prefix = "FXX";
 option php_namespace = "Foo\\V1Beta2";
 
+import "bar/v1beta1/bar.proto";
+import "bar/v2/bar.proto";
+
+message Deps {
+  bar.v1beta1.One v1beta1_one = 2;
+  bar.v2.One v2_one = 1;
+}
+
 enum EnumOne {
   ENUM_ONE_INVALID = 0;
   ENUM_ONE_ONE = 1;
diff --git a/internal/breaking/testdata/one/to/bar/v1beta1/bar.proto b/internal/breaking/testdata/one/to/bar/v1beta1/bar.proto
new file mode 100644
index 0000000..2a34b77
--- /dev/null
+++ b/internal/breaking/testdata/one/to/bar/v1beta1/bar.proto
@@ -0,0 +1,13 @@
+syntax = "proto3";
+
+package bar.v1beta1;
+
+option csharp_namespace = "Bar.V1Beta1";
+option go_package = "barv1beta1";
+option java_multiple_files = true;
+option java_outer_classname = "BarProto";
+option java_package = "com.bar.v1beta1";
+option objc_class_prefix = "BXX";
+option php_namespace = "Bar\\V1Beta1";
+
+message One {}
diff --git a/internal/breaking/testdata/one/to/bar/v2/bar.proto b/internal/breaking/testdata/one/to/bar/v2/bar.proto
new file mode 100644
index 0000000..9dc3dd7
--- /dev/null
+++ b/internal/breaking/testdata/one/to/bar/v2/bar.proto
@@ -0,0 +1,13 @@
+syntax = "proto3";
+
+package bar.v2;
+
+option csharp_namespace = "Bar.V2";
+option go_package = "barv2";
+option java_multiple_files = true;
+option java_outer_classname = "BarProto";
+option java_package = "com.bar.v2";
+option objc_class_prefix = "BXX";
+option php_namespace = "Bar\\V2";
+
+message One {}
diff --git a/internal/breaking/testdata/one/to/foo/v1/foo.proto b/internal/breaking/testdata/one/to/foo/v1/foo.proto
index 7509f10..7ef67c9 100644
--- a/internal/breaking/testdata/one/to/foo/v1/foo.proto
+++ b/internal/breaking/testdata/one/to/foo/v1/foo.proto
@@ -10,6 +10,14 @@ option java_package = "com.foo.v1";
 option objc_class_prefix = "FXX";
 option php_namespace = "Foo\\V1";
 
+import "bar/v1beta1/bar.proto";
+import "bar/v2/bar.proto";
+
+message Deps {
+  bar.v1beta1.One v1beta1_one = 2;
+  bar.v2.One v2_one = 1;
+}
+
 enum EnumOne {
   ENUM_ONE_INVALID = 0;
   ENUM_ONE_ONE = 1;
diff --git a/internal/breaking/testdata/one/to/foo/v1beta1/foo.proto b/internal/breaking/testdata/one/to/foo/v1beta1/foo.proto
index cf581fc..46a927c 100644
--- a/internal/breaking/testdata/one/to/foo/v1beta1/foo.proto
+++ b/internal/breaking/testdata/one/to/foo/v1beta1/foo.proto
@@ -10,6 +10,14 @@ option java_package = "com.foo.v1beta1";
 option objc_class_prefix = "FXX";
 option php_namespace = "Foo\\V1Beta1";
 
+import "bar/v1beta1/bar.proto";
+import "bar/v2/bar.proto";
+
+message Deps {
+  bar.v1beta1.One v1beta1_one = 2;
+  bar.v2.One v2_one = 1;
+}
+
 enum EnumOne {
   ENUM_ONE_INVALID = 0;
   ENUM_ONE_ONE = 1;
diff --git a/internal/cmd/testdata/lint/floats/foo/v1/foo.proto b/internal/cmd/testdata/lint/floats/foo/v1/foo.proto
index 702a515..0b3852f 100644
--- a/internal/cmd/testdata/lint/floats/foo/v1/foo.proto
+++ b/internal/cmd/testdata/lint/floats/foo/v1/foo.proto
@@ -14,9 +14,10 @@ message One {
   // This is a field we are ok with having floats.
   //
   // @suppresswarnings floats
-  double one = 1;
+  float one = 1;
   // @suppresswarnings floats
   float two = 2;
-  double three = 3;
-  double four = 4;
+  float three = 3;
+  float four = 4;
+  double five = 5;
 }