From dc702db3e96869912ad06a2befdde93cd0655e5f Mon Sep 17 00:00:00 2001 From: Mehul Kar Date: Mon, 19 Jun 2023 23:37:58 -0700 Subject: [PATCH] feat(logging): handle logging in github actions better --- cli/internal/run/real_run.go | 9 +++++++++ cli/internal/run/run.go | 8 ++++++++ cli/internal/util/run_opts.go | 3 +++ examples-tests/setup.sh | 13 +++++++++++++ turborepo-tests/e2e/index.ts | 13 +++++++++++++ turborepo-tests/integration/test.sh | 13 +++++++++++++ 6 files changed, 59 insertions(+) diff --git a/cli/internal/run/real_run.go b/cli/internal/run/real_run.go index 1d0bd570c2126..6e3ead5bafcf3 100644 --- a/cli/internal/run/real_run.go +++ b/cli/internal/run/real_run.go @@ -325,6 +325,10 @@ func (ec *execContext) exec(ctx gocontext.Context, packageTask *nodes.PackageTas WarnPrefix: prettyPrefix, } + if ec.rs.Opts.runOpts.IsGithubActions { + ui.Output(fmt.Sprintf("::group::%s", packageTask.OutputPrefix(ec.isSinglePackage))) + } + cacheStatus, err := taskCache.RestoreOutputs(ctx, prefixedUI, progressLogger) // It's safe to set the CacheStatus even if there's an error, because if there's @@ -413,6 +417,11 @@ func (ec *execContext) exec(ctx gocontext.Context, packageTask *nodes.PackageTas closeOutputs := func() error { var closeErrors []error + if ec.rs.Opts.runOpts.IsGithubActions { + // We don't use the prefixedUI here because the prefix in this case would include + // the ::group::, and we explicitly want to close the github group + ui.Output("::endgroup::") + } if err := logStreamerOut.Close(); err != nil { closeErrors = append(closeErrors, errors.Wrap(err, "log stdout")) diff --git a/cli/internal/run/run.go b/cli/internal/run/run.go index 460aef3bfabdc..dc81ba40d7e3d 100644 --- a/cli/internal/run/run.go +++ b/cli/internal/run/run.go @@ -9,6 +9,7 @@ import ( "github.com/vercel/turbo/cli/internal/analytics" "github.com/vercel/turbo/cli/internal/cache" + "github.com/vercel/turbo/cli/internal/ci" "github.com/vercel/turbo/cli/internal/cmdutil" "github.com/vercel/turbo/cli/internal/context" "github.com/vercel/turbo/cli/internal/core" @@ -128,6 +129,13 @@ func optsFromArgs(args *turbostate.ParsedArgsFromRust) (*Opts, error) { } func configureRun(base *cmdutil.CmdBase, opts *Opts, signalWatcher *signals.Watcher) *run { + if opts.runOpts.LogOrder == "auto" { + if name := ci.Constant(); name == "GITHUB_ACTIONS" { + opts.runOpts.LogOrder = "grouped" + opts.runOpts.IsGithubActions = true + } + } + processes := process.NewManager(base.Logger.Named("processes")) signalWatcher.AddOnClose(processes.Close) return &run{ diff --git a/cli/internal/util/run_opts.go b/cli/internal/util/run_opts.go index f81997931d1b4..8bcfcfd1c2d5e 100644 --- a/cli/internal/util/run_opts.go +++ b/cli/internal/util/run_opts.go @@ -55,4 +55,7 @@ type RunOpts struct { Summarize bool ExperimentalSpaceID string + + // Whether this run is in Github Actions + IsGithubActions bool } diff --git a/examples-tests/setup.sh b/examples-tests/setup.sh index aadbfdcf6d141..8c1160e23ed24 100644 --- a/examples-tests/setup.sh +++ b/examples-tests/setup.sh @@ -72,3 +72,16 @@ fi [ ! -d .git ] || rm -rf .git "$MONOREPO_ROOT_DIR/turborepo-tests/helpers/setup_git.sh" "${TARGET_DIR}" "false" + +# We set this explicitly to stream, so we can lock into to streaming logs (i.e. not "auto") behavior. +# +# We do this because when these tests are invoked in CI (through .github/actions/test.yml), they will +# inherit the GITHUB_ACTIONS=true env var, and each of the `turbo run` invocations into behavior +# we do not want. Since prysk mainly tests log output, this extra behavior will break all the tests +# and can be unpredictable over time, as we make "auto" do more magic. +# +# Note: since these tests are invoked _through_ turbo, the ideal setup would be to pass --env-mode=strict +# so we can prevent the `GITHUB_ACTIONS` env var from being passed down here from the top level turbo. +# But as of now, this breaks our tests (and I'm not sure why). If we make that work, we can remove this +# explicit locking of log order. See PR attempt here: https://github.com/vercel/turbo/pull/5324 +export TURBO_LOG_ORDER=stream diff --git a/turborepo-tests/e2e/index.ts b/turborepo-tests/e2e/index.ts index 0dba894bca48e..ec3fafe6af8c3 100644 --- a/turborepo-tests/e2e/index.ts +++ b/turborepo-tests/e2e/index.ts @@ -8,6 +8,19 @@ import { Monorepo } from "./monorepo"; import path from "path"; import * as fs from "fs"; +// We set this explicitly to stream, so we can lock into to streaming logs (i.e. not "auto") behavior. +// +// We do this because when these tests are invoked in CI (through .github/actions/test.yml), they will +// inherit the GITHUB_ACTIONS=true env var, and each of the `turbo run` invocations into behavior +// we do not want. Since prysk mainly tests log output, this extra behavior will break all the tests +// and can be unpredictable over time, as we make "auto" do more magic. +// +// Note: since these tests are invoked _through_ turbo, the ideal setup would be to pass --env-mode=strict +// so we can prevent the `GITHUB_ACTIONS` env var from being passed down here from the top level turbo. +// But as of now, this breaks our tests (and I'm not sure why). If we make that work, we can remove this +// explicit locking of log order. See PR attempt here: https://github.com/vercel/turbo/pull/5324 +process.env.TURBO_LOG_ORDER = "stream"; + const basicPipeline = { pipeline: { test: { diff --git a/turborepo-tests/integration/test.sh b/turborepo-tests/integration/test.sh index 00988b8199026..bf85e044f8dfb 100755 --- a/turborepo-tests/integration/test.sh +++ b/turborepo-tests/integration/test.sh @@ -1,5 +1,18 @@ #!/bin/bash +# We set this explicitly to stream, so we can lock into to streaming logs (i.e. not "auto") behavior. +# +# We do this because when these tests are invoked in CI (through .github/actions/test.yml), they will +# inherit the GITHUB_ACTIONS=true env var, and each of the `turbo run` invocations into behavior +# we do not want. Since prysk mainly tests log output, this extra behavior will break all the tests +# and can be unpredictable over time, as we make "auto" do more magic. +# +# Note: since these tests are invoked _through_ turbo, the ideal setup would be to pass --env-mode=strict +# so we can prevent the `GITHUB_ACTIONS` env var from being passed down here from the top level turbo. +# But as of now, this breaks our tests (and I'm not sure why). If we make that work, we can remove this +# explicit locking of log order. See PR attempt here: https://github.com/vercel/turbo/pull/5324 +export TURBO_LOG_ORDER=stream + if [ "$1" = "" ]; then .cram_env/bin/prysk --shell="$(which bash)" tests else