From a3cb2b396f9bc30355c6125eeeb67ee7256cf9f7 Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Wed, 10 Mar 2021 09:15:22 +0000 Subject: [PATCH] Remove duplicate sdk.TestHelper As part of the work on the new fixture mechanism some of the TestHelper functionality was moved into the android/fixture.go package. This moves the rest and removes the now duplicated TestHelper from the sdk package. Also removed some unnecessary & operators. Bug: 181070625 Test: m nothing Change-Id: Ia09a5d05e4fab3a4e28cf44b2d947a33541e3925 --- android/fixture.go | 29 +++++++++++++++++ sdk/bp_test.go | 14 ++++----- sdk/sdk_test.go | 4 +-- sdk/testing.go | 78 +++++++--------------------------------------- 4 files changed, 50 insertions(+), 75 deletions(-) diff --git a/android/fixture.go b/android/fixture.go index 2085e43ed..925161524 100644 --- a/android/fixture.go +++ b/android/fixture.go @@ -484,6 +484,18 @@ func (h *TestHelper) AssertStringEquals(message string, expected string, actual } } +// AssertErrorMessageEquals checks if the error is not nil and has the expected message. If it does +// not then this reports an error prefixed with the supplied message and including a reason for why +// it failed. +func (h *TestHelper) AssertErrorMessageEquals(message string, expected string, actual error) { + h.Helper() + if actual == nil { + h.Errorf("Expected error but was nil") + } else if actual.Error() != expected { + h.Errorf("%s: expected %s, actual %s", message, expected, actual.Error()) + } +} + // AssertTrimmedStringEquals checks if the expected and actual values are the same after trimming // leading and trailing spaces from them both. If they are not then it reports an error prefixed // with the supplied message and including a reason for why it failed. @@ -538,6 +550,23 @@ func (h *TestHelper) AssertDeepEquals(message string, expected interface{}, actu } } +// AssertPanic checks that the supplied function panics as expected. +func (h *TestHelper) AssertPanic(message string, funcThatShouldPanic func()) { + h.Helper() + panicked := false + func() { + defer func() { + if x := recover(); x != nil { + panicked = true + } + }() + funcThatShouldPanic() + }() + if !panicked { + h.Error(message) + } +} + // Struct to allow TestResult to embed a *TestContext and allow call forwarding to its methods. type testContext struct { *TestContext diff --git a/sdk/bp_test.go b/sdk/bp_test.go index e1edc5131..a7164a5e9 100644 --- a/sdk/bp_test.go +++ b/sdk/bp_test.go @@ -54,7 +54,7 @@ func propertyStructFixture() interface{} { return str } -func checkPropertySetFixture(h *TestHelper, val interface{}, hasTags bool) { +func checkPropertySetFixture(h android.TestHelper, val interface{}, hasTags bool) { set := val.(*bpPropertySet) h.AssertDeepEquals("wrong x value", "taxi", set.getValue("x")) h.AssertDeepEquals("wrong y value", 1729, set.getValue("y")) @@ -73,7 +73,7 @@ func checkPropertySetFixture(h *TestHelper, val interface{}, hasTags bool) { } func TestAddPropertySimple(t *testing.T) { - h := &TestHelper{t} + h := android.TestHelper{t} set := newPropertySet() for name, val := range map[string]interface{}{ "x": "taxi", @@ -92,7 +92,7 @@ func TestAddPropertySimple(t *testing.T) { } func TestAddPropertySubset(t *testing.T) { - h := &TestHelper{t} + h := android.TestHelper{t} getFixtureMap := map[string]func() interface{}{ "property set": propertySetFixture, "property struct": propertyStructFixture, @@ -139,7 +139,7 @@ func TestAddPropertySubset(t *testing.T) { } func TestAddPropertySetNew(t *testing.T) { - h := &TestHelper{t} + h := android.TestHelper{t} set := newPropertySet() subset := set.AddPropertySet("sub") subset.AddProperty("new", "d^^b") @@ -147,7 +147,7 @@ func TestAddPropertySetNew(t *testing.T) { } func TestAddPropertySetExisting(t *testing.T) { - h := &TestHelper{t} + h := android.TestHelper{t} set := propertySetFixture().(*bpPropertySet) subset := set.AddPropertySet("sub") subset.AddProperty("new", "d^^b") @@ -181,7 +181,7 @@ func (t removeFredTransformation) transformPropertySetAfterContents(name string, func TestTransformRemoveProperty(t *testing.T) { - helper := &TestHelper{t} + helper := android.TestHelper{t} set := newPropertySet() set.AddProperty("name", "name") @@ -196,7 +196,7 @@ func TestTransformRemoveProperty(t *testing.T) { func TestTransformRemovePropertySet(t *testing.T) { - helper := &TestHelper{t} + helper := android.TestHelper{t} set := newPropertySet() set.AddProperty("name", "name") diff --git a/sdk/sdk_test.go b/sdk/sdk_test.go index c4dc41beb..8f885a10b 100644 --- a/sdk/sdk_test.go +++ b/sdk/sdk_test.go @@ -390,7 +390,7 @@ func TestCommonValueOptimization(t *testing.T) { extractor := newCommonValueExtractor(common) - h := TestHelper{t} + h := android.TestHelper{t} err := extractor.extractCommonProperties(common, structs) h.AssertDeepEquals("unexpected error", nil, err) @@ -465,7 +465,7 @@ func TestCommonValueOptimization_InvalidArchSpecificVariants(t *testing.T) { extractor := newCommonValueExtractor(common) - h := TestHelper{t} + h := android.TestHelper{t} err := extractor.extractCommonProperties(common, structs) h.AssertErrorMessageEquals("unexpected error", `field "S_Common" is not tagged as "arch_variant" but has arch specific properties: diff --git a/sdk/testing.go b/sdk/testing.go index 7a2540a27..c401c867e 100644 --- a/sdk/testing.go +++ b/sdk/testing.go @@ -19,7 +19,6 @@ import ( "io/ioutil" "os" "path/filepath" - "reflect" "strings" "testing" @@ -141,7 +140,7 @@ func runTests(t *testing.T, ctx *android.TestContext, config android.Config) *te _, errs = ctx.PrepareBuildActions(config) android.FailIfErrored(t, errs) return &testSdkResult{ - TestHelper: TestHelper{t: t}, + TestHelper: android.TestHelper{T: t}, ctx: ctx, config: config, } @@ -185,63 +184,10 @@ func pathsToStrings(paths android.Paths) []string { return ret } -// Provides general test support. -type TestHelper struct { - t *testing.T -} - -func (h *TestHelper) AssertStringEquals(message string, expected string, actual string) { - h.t.Helper() - if actual != expected { - h.t.Errorf("%s: expected %s, actual %s", message, expected, actual) - } -} - -func (h *TestHelper) AssertErrorMessageEquals(message string, expected string, actual error) { - h.t.Helper() - if actual == nil { - h.t.Errorf("Expected error but was nil") - } else if actual.Error() != expected { - h.t.Errorf("%s: expected %s, actual %s", message, expected, actual.Error()) - } -} - -func (h *TestHelper) AssertTrimmedStringEquals(message string, expected string, actual string) { - h.t.Helper() - expected = strings.TrimSpace(expected) - actual = strings.TrimSpace(actual) - if actual != expected { - h.t.Errorf("%s: expected:\n%s\nactual:\n%s\n", message, expected, actual) - } -} - -func (h *TestHelper) AssertDeepEquals(message string, expected interface{}, actual interface{}) { - h.t.Helper() - if !reflect.DeepEqual(actual, expected) { - h.t.Errorf("%s: expected %#v, actual %#v", message, expected, actual) - } -} - -func (h *TestHelper) AssertPanic(message string, funcThatShouldPanic func()) { - h.t.Helper() - panicked := false - func() { - defer func() { - if x := recover(); x != nil { - panicked = true - } - }() - funcThatShouldPanic() - }() - if !panicked { - h.t.Error(message) - } -} - // Encapsulates result of processing an SDK definition. Provides support for // checking the state of the build structures. type testSdkResult struct { - TestHelper + android.TestHelper ctx *android.TestContext config android.Config } @@ -295,7 +241,7 @@ func (r *testSdkResult) getSdkSnapshotBuildInfo(sdk *sdk) *snapshotBuildInfo { info.intermediateZip = info.outputZip mergeInput := android.NormalizePathForTesting(bp.Input) if info.intermediateZip != mergeInput { - r.t.Errorf("Expected intermediate zip %s to be an input to merge zips but found %s instead", + r.Errorf("Expected intermediate zip %s to be an input to merge zips but found %s instead", info.intermediateZip, mergeInput) } @@ -328,7 +274,7 @@ func (r *testSdkResult) ModuleForTests(name string, variant string) android.Test // Allows each test to customize what is checked without duplicating lots of code // or proliferating check methods of different flavors. func (r *testSdkResult) CheckSnapshot(name string, dir string, checkers ...snapshotBuildInfoChecker) { - r.t.Helper() + r.Helper() // The sdk CommonOS variant is always responsible for generating the snapshot. variant := android.CommonOS.Name @@ -358,7 +304,7 @@ func (r *testSdkResult) CheckSnapshot(name string, dir string, checkers ...snaps } // Process the generated bp file to make sure it is valid. - testSdkWithFs(r.t, snapshotBuildInfo.androidBpContents, fs) + testSdkWithFs(r.T, snapshotBuildInfo.androidBpContents, fs) } type snapshotBuildInfoChecker func(info *snapshotBuildInfo) @@ -368,7 +314,7 @@ type snapshotBuildInfoChecker func(info *snapshotBuildInfo) // Both the expected and actual string are both trimmed before comparing. func checkAndroidBpContents(expected string) snapshotBuildInfoChecker { return func(info *snapshotBuildInfo) { - info.r.t.Helper() + info.r.Helper() info.r.AssertTrimmedStringEquals("Android.bp contents do not match", expected, info.androidBpContents) } } @@ -380,7 +326,7 @@ func checkAndroidBpContents(expected string) snapshotBuildInfoChecker { // Both the expected and actual string are both trimmed before comparing. func checkUnversionedAndroidBpContents(expected string) snapshotBuildInfoChecker { return func(info *snapshotBuildInfo) { - info.r.t.Helper() + info.r.Helper() info.r.AssertTrimmedStringEquals("unversioned Android.bp contents do not match", expected, info.androidUnversionedBpContents) } } @@ -395,7 +341,7 @@ func checkUnversionedAndroidBpContents(expected string) snapshotBuildInfoChecker // Both the expected and actual string are both trimmed before comparing. func checkVersionedAndroidBpContents(expected string) snapshotBuildInfoChecker { return func(info *snapshotBuildInfo) { - info.r.t.Helper() + info.r.Helper() info.r.AssertTrimmedStringEquals("versioned Android.bp contents do not match", expected, info.androidVersionedBpContents) } } @@ -407,14 +353,14 @@ func checkVersionedAndroidBpContents(expected string) snapshotBuildInfoChecker { // before comparing. func checkAllCopyRules(expected string) snapshotBuildInfoChecker { return func(info *snapshotBuildInfo) { - info.r.t.Helper() + info.r.Helper() info.r.AssertTrimmedStringEquals("Incorrect copy rules", expected, info.copyRules) } } func checkAllOtherCopyRules(expected string) snapshotBuildInfoChecker { return func(info *snapshotBuildInfo) { - info.r.t.Helper() + info.r.Helper() info.r.AssertTrimmedStringEquals("Incorrect copy rules", expected, info.otherCopyRules) } } @@ -422,9 +368,9 @@ func checkAllOtherCopyRules(expected string) snapshotBuildInfoChecker { // Check that the specified paths match the list of zips to merge with the intermediate zip. func checkMergeZips(expected ...string) snapshotBuildInfoChecker { return func(info *snapshotBuildInfo) { - info.r.t.Helper() + info.r.Helper() if info.intermediateZip == "" { - info.r.t.Errorf("No intermediate zip file was created") + info.r.Errorf("No intermediate zip file was created") } info.r.AssertDeepEquals("mismatching merge zip files", expected, info.mergeZips)