From 7aaabcefdfa3e0e09d9b888543cc300d9114a9bb Mon Sep 17 00:00:00 2001 From: Keepers Date: Tue, 23 May 2023 14:21:29 -0600 Subject: [PATCH] check for log file location in artifacts (#3470) ensure test artifacts have log files --- #### Does this PR need a docs update or release note? - [x] :no_entry: No #### Type of change - [x] :robot: Supportability/Tests #### Test Plan - [x] :green_heart: E2E --- .github/workflows/ci.yml | 10 ++-- .github/workflows/load_test.yml | 4 +- .github/workflows/nightly_test.yml | 4 +- .github/workflows/sanity-test.yaml | 4 +- src/cli/cli.go | 4 +- .../connector/exchange/restore_test.go | 17 ++++--- src/internal/tester/cli.go | 34 -------------- src/internal/tester/context.go | 47 +++++++++++++++++++ src/pkg/logger/logger.go | 11 +++-- src/pkg/logger/logger_test.go | 2 +- 10 files changed, 77 insertions(+), 60 deletions(-) create mode 100644 src/internal/tester/context.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e9edc7e33..4cbb065e6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -137,7 +137,7 @@ jobs: AZURE_CLIENT_ID_NAME: ${{ needs.SetM365App.outputs.client_id_env }} AZURE_CLIENT_SECRET_NAME: ${{ needs.SetM365App.outputs.client_secret_env }} CLIENT_APP_SLOT: ${{ needs.SetM365App.outputs.client_app_slot }} - CORSO_LOG_FILE: ./src/testlog/suite-testlogging.log + CORSO_LOG_FILE: ${{ github.workspace }}/src/testlog/run-ci.log LOG_GRAPH_REQUESTS: true steps: - uses: actions/checkout@v3 @@ -181,7 +181,7 @@ jobs: -p 1 \ -timeout 15m \ ./... \ - 2>&1 | tee ./testlog/gotest.log | gotestfmt -hide successful-tests + 2>&1 | tee ./testlog/gotest-ci.log | gotestfmt -hide successful-tests # Upload the original go test output as an artifact for later review. - name: Upload test log @@ -202,7 +202,7 @@ jobs: run: working-directory: src env: - CORSO_LOG_FILE: ./src/testlog/unit-testlogging.log + CORSO_LOG_FILE: ${{ github.workspace }}/src/testlog/run-unit.log LOG_GRAPH_REQUESTS: true steps: - uses: actions/checkout@v3 @@ -256,7 +256,7 @@ jobs: run: working-directory: src env: - CORSO_LOG_FILE: ./src/testlog/fork-testlogging.log + CORSO_LOG_FILE: ${{ github.workspace }}/testlog/run-fork.log LOG_GRAPH_REQUESTS: true steps: - name: Fail check if not repository_dispatch @@ -324,7 +324,7 @@ jobs: -v \ -timeout 15m \ ./... \ - 2>&1 | tee ./testlog/gotest.log | gotestfmt -hide successful-tests + 2>&1 | tee ./testlog/gotest-fork.log | gotestfmt -hide successful-tests # Upload the original go test log as an artifact for later review. - name: Upload test log diff --git a/.github/workflows/load_test.yml b/.github/workflows/load_test.yml index 9241b3d8f..4a45b8029 100644 --- a/.github/workflows/load_test.yml +++ b/.github/workflows/load_test.yml @@ -55,7 +55,7 @@ jobs: AZURE_CLIENT_SECRET: ${{ secrets.AZURE_CLIENT_SECRET }} AZURE_TENANT_ID: ${{ secrets.AZURE_TENANT_ID }} CORSO_LOAD_TESTS: true - CORSO_LOG_FILE: ./src/test_results/testlogging.log + CORSO_LOG_FILE: ${{ github.workspace }}/test_results/run-load.log CORSO_M365_LOAD_TEST_USER_ID: ${{ secrets.CORSO_M365_LOAD_TEST_USER_ID }} CORSO_M365_LOAD_TEST_ORG_USERS: ${{ secrets.CORSO_M365_LOAD_TEST_ORG_USERS }} CORSO_PASSPHRASE: ${{ secrets.CORSO_PASSPHRASE }} @@ -75,7 +75,7 @@ jobs: -trace=trace.out \ -outputdir=test_results \ ./pkg/repository/repository_load_test.go \ - 2>&1 | tee ./test_results/goloadtest.log | gotestfmt -hide successful-tests + 2>&1 | tee ./test_results/gotest-load.log | gotestfmt -hide successful-tests # generate new entries to roll into the next load test # only runs if the test was successful diff --git a/.github/workflows/nightly_test.yml b/.github/workflows/nightly_test.yml index a1022dd4b..d1f61d167 100644 --- a/.github/workflows/nightly_test.yml +++ b/.github/workflows/nightly_test.yml @@ -125,7 +125,7 @@ jobs: CORSO_M365_TEST_USER_ID: ${{ vars.CORSO_M365_TEST_USER_ID }} CORSO_SECONDARY_M365_TEST_USER_ID: ${{ vars.CORSO_SECONDARY_M365_TEST_USER_ID }} CORSO_PASSPHRASE: ${{ secrets.INTEGRATION_TEST_CORSO_PASSPHRASE }} - CORSO_LOG_FILE: ./src/testlog/testlogging.log + CORSO_LOG_FILE: ${{ github.workspace }}/testlog/run-nightly.log LOG_GRAPH_REQUESTS: true run: | set -euo pipefail @@ -136,7 +136,7 @@ jobs: -failfast \ -p 1 \ -timeout 15m \ - ./... 2>&1 | tee ./testlog/gotest.log | gotestfmt -hide successful-tests + ./... 2>&1 | tee ./testlog/gotest-nightly.log | gotestfmt -hide successful-tests ########################################################################################################################################## diff --git a/.github/workflows/sanity-test.yaml b/.github/workflows/sanity-test.yaml index 4a575fab8..3480f3200 100644 --- a/.github/workflows/sanity-test.yaml +++ b/.github/workflows/sanity-test.yaml @@ -37,7 +37,7 @@ jobs: # re-used values # don't forget: return to Corso_Test_Sanity_ CORSO_LOG_DIR: testlog - CORSO_LOG_FILE: testlog/testlogging.log + CORSO_LOG_FILE: ${{ github.workspace }}/src/testlog/run-sanity.log RESTORE_DEST_PFX: Corso_Test_Sanity_ TEST_RESULT: test_results TEST_USER: ${{ github.event.inputs.user != '' && github.event.inputs.user || secrets.CORSO_M365_TEST_USER_ID }} @@ -328,7 +328,7 @@ jobs: uses: actions/upload-artifact@v3 with: name: sanity-test-log - path: ${{ env.WORKING_DIR }}/${{ env.CORSO_LOG_DIR }}/ + path: ${{ github.workspace }}/${{ env.CORSO_LOG_DIR }}/ if-no-files-found: error retention-days: 14 diff --git a/src/cli/cli.go b/src/cli/cli.go index 4a851b3c2..eb1276cb5 100644 --- a/src/cli/cli.go +++ b/src/cli/cli.go @@ -56,8 +56,8 @@ func preRun(cc *cobra.Command, args []string) error { "corso", "env", "help", "backup", "details", "list", "restore", "delete", "repo", "init", "connect", } - if len(logger.LogFile) > 0 && !slices.Contains(avoidTheseCommands, cc.Use) { - print.Info(ctx, "Logging to file: "+logger.LogFile) + if len(logger.ResolvedLogFile) > 0 && !slices.Contains(avoidTheseCommands, cc.Use) { + print.Infof(ctx, "Logging to file: %s", logger.ResolvedLogFile) } avoidTheseDescription := []string{ diff --git a/src/internal/connector/exchange/restore_test.go b/src/internal/connector/exchange/restore_test.go index de67ab8e6..90bf95b8a 100644 --- a/src/internal/connector/exchange/restore_test.go +++ b/src/internal/connector/exchange/restore_test.go @@ -20,23 +20,22 @@ import ( "github.com/alcionai/corso/src/pkg/services/m365/api" ) -type ExchangeRestoreSuite struct { +type RestoreIntgSuite struct { tester.Suite gs graph.Servicer credentials account.M365Config ac api.Client } -func TestExchangeRestoreSuite(t *testing.T) { - suite.Run(t, &ExchangeRestoreSuite{ +func TestRestoreIntgSuite(t *testing.T) { + suite.Run(t, &RestoreIntgSuite{ Suite: tester.NewIntegrationSuite( t, - [][]string{tester.M365AcctCredEnvs}, - ), + [][]string{tester.M365AcctCredEnvs}), }) } -func (suite *ExchangeRestoreSuite) SetupSuite() { +func (suite *RestoreIntgSuite) SetupSuite() { t := suite.T() a := tester.NewM365Account(t) @@ -58,7 +57,7 @@ func (suite *ExchangeRestoreSuite) SetupSuite() { // TestRestoreContact ensures contact object can be created, placed into // the Corso Folder. The function handles test clean-up. -func (suite *ExchangeRestoreSuite) TestRestoreContact() { +func (suite *RestoreIntgSuite) TestRestoreContact() { ctx, flush := tester.NewContext() defer flush() @@ -92,7 +91,7 @@ func (suite *ExchangeRestoreSuite) TestRestoreContact() { // TestRestoreEvent verifies that event object is able to created // and sent into the test account of the Corso user in the newly created Corso Calendar -func (suite *ExchangeRestoreSuite) TestRestoreEvent() { +func (suite *RestoreIntgSuite) TestRestoreEvent() { ctx, flush := tester.NewContext() defer flush() @@ -153,7 +152,7 @@ type containerDeleter interface { } // TestRestoreExchangeObject verifies path.Category usage for restored objects -func (suite *ExchangeRestoreSuite) TestRestoreExchangeObject() { +func (suite *RestoreIntgSuite) TestRestoreExchangeObject() { t := suite.T() a := tester.NewM365Account(t) m365, err := a.M365Config() diff --git a/src/internal/tester/cli.go b/src/internal/tester/cli.go index 925233d2e..bf63228d9 100644 --- a/src/internal/tester/cli.go +++ b/src/internal/tester/cli.go @@ -1,16 +1,13 @@ package tester import ( - "context" "fmt" - "os" "time" "github.com/google/uuid" "github.com/spf13/cobra" "github.com/alcionai/corso/src/internal/common/dttm" - "github.com/alcionai/corso/src/pkg/logger" ) // StubRootCmd builds a stub cobra command to be used as @@ -32,34 +29,3 @@ func StubRootCmd(args ...string) *cobra.Command { return c } - -func NewContext() (context.Context, func()) { - level := logger.LLInfo - format := logger.LFText - - for _, a := range os.Args { - if a == "-test.v=true" { - level = logger.LLDebug - } - } - - ls := logger.Settings{ - Level: level, - Format: format, - } - - //nolint:forbidigo - ctx, _ := logger.CtxOrSeed(context.Background(), ls) - - return ctx, func() { logger.Flush(ctx) } -} - -func WithContext(ctx context.Context) (context.Context, func()) { - ls := logger.Settings{ - Level: logger.LLDebug, - Format: logger.LFText, - } - ctx, _ = logger.CtxOrSeed(ctx, ls) - - return ctx, func() { logger.Flush(ctx) } -} diff --git a/src/internal/tester/context.go b/src/internal/tester/context.go new file mode 100644 index 000000000..ac95d8f5b --- /dev/null +++ b/src/internal/tester/context.go @@ -0,0 +1,47 @@ +package tester + +import ( + "context" + "os" + + "github.com/alcionai/clues" + "github.com/google/uuid" + + "github.com/alcionai/corso/src/pkg/logger" +) + +func NewContext() (context.Context, func()) { + level := logger.LLInfo + format := logger.LFText + + for _, a := range os.Args { + if a == "-test.v=true" { + level = logger.LLDebug + } + } + + ls := logger.Settings{ + Level: level, + Format: format, + } + + //nolint:forbidigo + ctx, _ := logger.CtxOrSeed(context.Background(), ls) + + // ensure logs can be easily associated with each test + // todo: replace with test name. starting off with + // uuid to avoid million-line PR change. + ctx = clues.Add(ctx, "test_name", uuid.NewString()) + + return ctx, func() { logger.Flush(ctx) } +} + +func WithContext(ctx context.Context) (context.Context, func()) { + ls := logger.Settings{ + Level: logger.LLDebug, + Format: logger.LFText, + } + ctx, _ = logger.CtxOrSeed(ctx, ls) + + return ctx, func() { logger.Flush(ctx) } +} diff --git a/src/pkg/logger/logger.go b/src/pkg/logger/logger.go index f6af8ffce..48c23e5af 100644 --- a/src/pkg/logger/logger.go +++ b/src/pkg/logger/logger.go @@ -71,8 +71,8 @@ var ( ReadableLogsFV bool MaskSensitiveDataFV bool - LogFile string // logFileFV after processing - piiHandling string // piiHandling after MaskSensitiveDataFV processing + ResolvedLogFile string // logFileFV after processing + piiHandling string // piiHandling after MaskSensitiveDataFV processing ) const ( @@ -184,7 +184,7 @@ func PreloadLoggingFlags(args []string) Settings { } set.File = GetLogFile(lffv) - LogFile = set.File + ResolvedLogFile = set.File // retrieve the user's preferred PII handling algorithm // defaults to "plaintext" @@ -203,6 +203,10 @@ func PreloadLoggingFlags(args []string) Settings { // GetLogFile parses the log file. Uses the provided value, if populated, // then falls back to the env var, and then defaults to stderr. func GetLogFile(logFileFlagVal string) string { + if len(ResolvedLogFile) > 0 { + return ResolvedLogFile + } + r := logFileFlagVal // if not specified, attempt to fall back to env declaration. @@ -261,6 +265,7 @@ func (s Settings) EnsureDefaults() Settings { if len(set.File) == 0 { set.File = GetLogFile("") + ResolvedLogFile = set.File } return set diff --git a/src/pkg/logger/logger_test.go b/src/pkg/logger/logger_test.go index 910b546b0..0204c3a69 100644 --- a/src/pkg/logger/logger_test.go +++ b/src/pkg/logger/logger_test.go @@ -38,7 +38,7 @@ func (suite *LoggerUnitSuite) TestAddLoggingFlags() { // empty assertion here, instead of matching "log-file", because the LogFile // var isn't updated by running the command (this is expected and correct), // while the logFileFV remains unexported. - assert.Empty(t, logger.LogFile, logger.LogFileFN) + assert.Empty(t, logger.ResolvedLogFile, logger.LogFileFN) }, }