Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix color handling, generate the summary with JS, improve CI #1975

Merged
merged 16 commits into from
May 18, 2021
Merged

Fix color handling, generate the summary with JS, improve CI #1975

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
4037eab
Improve TTY detection for stdout and stderr
na-- Apr 16, 2021
73b4a9b
Differentiate between stderr and stdout TTY detection for loggers
na-- Apr 16, 2021
b02fe7b
Do not use global variables and state to colorize some UI elements
na-- Apr 16, 2021
479332f
Do not highlight the output of k6 control subcommands
na-- Apr 16, 2021
b6de30b
Fix discrepancies in the the end-of-test summary tests
na-- Apr 16, 2021
f0b96c4
Propagate the current UI state through to handleSummary()
na-- Apr 16, 2021
580b054
Add testRunDurationMs to the handleSummary() state data
na-- May 14, 2021
c03a01b
Bump the minimum required Go version to 1.16 and test xk6 with tip
na-- Apr 16, 2021
dad3e26
Run the test suite with the Go tip version as well
na-- Apr 16, 2021
1c0f101
Generate the end-of-test text summary with JS instead of with Go
na-- Apr 16, 2021
2810d83
Drop the old Go code that was used for the end-of-test summary
na-- Apr 16, 2021
c2e4d33
Remove the hack with forcing stdout and stderr to be non-colorable
na-- Apr 16, 2021
c7e8a7b
Use go:embed for babel, drop github.com/GeertJohan/go.rice
na-- Apr 16, 2021
6f6bfac
Move the summary wrapper JS code to a new file and use it with go:embed
na-- May 18, 2021
6778f12
Make the CI config more robust
na-- May 18, 2021
0dd3012
Fix error typo in HandleSummary()
May 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions .github/workflows/all.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,12 @@ jobs:

test:
strategy:
fail-fast: false
matrix:
go-version: [1.15.x]
platform: [ubuntu-latest, windows-latest]
runs-on: ${{ matrix.platform }}
if: github.ref == 'skipped' #TODO: re-enable this when Go 1.17 comes out?
steps:
- name: Checkout code
uses: actions/checkout@v2
Expand All @@ -84,8 +86,36 @@ jobs:
fi
go test "${args[@]}" -timeout 800s ./...

test-tip:
runs-on: ubuntu-latest
continue-on-error: true
steps:
- name: Checkout code
uses: actions/checkout@v2
- name: Install Go
uses: actions/setup-go@v2
with:
go-version: 1.x
- name: Install Go tip
run: |
go install golang.org/dl/gotip@latest
gotip download
echo "GOROOT=$HOME/sdk/gotip" >> "$GITHUB_ENV"
echo "GOPATH=$HOME/go" >> "$GITHUB_ENV"
echo "$HOME/go/bin" >> "$GITHUB_PATH"
echo "$HOME/sdk/gotip/bin" >> "$GITHUB_PATH"
- name: Run tests
run: |
set -x
which go
go version
export GOMAXPROCS=2
args=("-p" "2" "-race")
go test "${args[@]}" -timeout 800s ./...
imiric marked this conversation as resolved.
Show resolved Hide resolved

test-cov:
strategy:
fail-fast: false
matrix:
go-version: [1.16.x]
platform: [ubuntu-latest, windows-latest]
Expand Down
18 changes: 16 additions & 2 deletions .github/workflows/xk6.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ jobs:
test-xk6:
strategy:
matrix:
go-version: [1.15.x, 1.16.x]
go: [stable, tip]
platform: [ubuntu-latest, windows-latest, macos-latest]
runs-on: ubuntu-latest
steps:
Expand All @@ -24,10 +24,24 @@ jobs:
- name: Install Go
uses: actions/setup-go@v2
with:
go-version: ${{ matrix.go-version }}
go-version: 1.x
- name: Install Go tip
if: matrix.go == 'tip'
run: |
go install golang.org/dl/gotip@latest
gotip download
echo "GOROOT=$HOME/sdk/gotip" >> "$GITHUB_ENV"
echo "GOPATH=$HOME/go" >> "$GITHUB_ENV"
echo "$HOME/go/bin" >> "$GITHUB_PATH"
echo "$HOME/sdk/gotip/bin" >> "$GITHUB_PATH"
- name: Run tests
run: |
set -x
which go
go version

export CGO_ENABLED=0

COMMIT_ID="${{ github.event.pull_request.head.sha }}"
if [[ "$COMMIT_ID" == "" ]]; then
COMMIT_ID="$(git rev-parse HEAD)" # branch, not PR merge commit
Expand Down
2 changes: 0 additions & 2 deletions .golangci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
run:
deadline: 5m
skip-files:
- "rice-box.go$"

issues:
# Maximum issues count per one linter. Set to 0 to disable. Default is 50.
Expand Down
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ If there isn't an official package for your operating system or architecture, or

### Build from source

k6 is written in Go, so it's just a single statically-linked executable and very easy to build and distribute. To build from source you need **[Git](https://git-scm.com/downloads)** and **[Go](https://golang.org/doc/install)** (1.12 or newer). Follow these instructions:
k6 is written in Go, so it's just a single statically-linked executable and very easy to build and distribute. To build from source you need **[Git](https://git-scm.com/downloads)** and **[Go](https://golang.org/doc/install)** (1.16 or newer). Follow these instructions:

- Run `go get go.k6.io/k6` which will:
- git clone the repo and put the source in `$GOPATH/src/go.k6.io/k6`
Expand Down
12 changes: 8 additions & 4 deletions cmd/cloud.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
"syscall"
"time"

"github.com/fatih/color"
"github.com/sirupsen/logrus"
"github.com/spf13/afero"
"github.com/spf13/cobra"
Expand All @@ -43,7 +44,6 @@ import (
"go.k6.io/k6/lib"
"go.k6.io/k6/lib/consts"
"go.k6.io/k6/loader"
"go.k6.io/k6/ui"
"go.k6.io/k6/ui/pb"
)

Expand Down Expand Up @@ -83,7 +83,7 @@ This will execute the test on the k6 cloud service. Use "k6 login cloud" to auth
}
}
// TODO: disable in quiet mode?
_, _ = BannerColor.Fprintf(stdout, "\n%s\n\n", consts.Banner())
_, _ = fmt.Fprintf(stdout, "\n%s\n\n", getBanner(noColor || !stdoutTTY))

progressBar := pb.New(
pb.WithConstLeft("Init"),
Expand Down Expand Up @@ -242,7 +242,10 @@ This will execute the test on the k6 cloud service. Use "k6 login cloud" to auth
}
testURL := cloudapi.URLForResults(refID, cloudConfig)
executionPlan := derivedConf.Scenarios.GetFullExecutionRequirements(et)
printExecutionDescription("cloud", filename, testURL, derivedConf, et, executionPlan, nil)
printExecutionDescription(
"cloud", filename, testURL, derivedConf, et,
executionPlan, nil, noColor || !stdoutTTY,
)

modifyAndPrintBar(
progressBar,
Expand Down Expand Up @@ -330,7 +333,8 @@ This will execute the test on the k6 cloud service. Use "k6 login cloud" to auth
return ExitCode{error: errors.New("Test progress error"), Code: cloudFailedToGetProgressErrorCode}
}

fprintf(stdout, " test status: %s\n", ui.ValueColor.Sprint(testProgress.RunStatusText))
valueColor := getColor(noColor || !stdoutTTY, color.FgCyan)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given how now you repeat color.FgCyan a lot I would argue it should probably be a constant somewhere ... probably named valueColorColor :P

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's already a constant 😛 It's used in like 5 places across 2 modules, and not always for values (e.g. the banner), so I don't want to define a constant for that.

fprintf(stdout, " test status: %s\n", valueColor.Sprint(testProgress.RunStatusText))

if testProgress.ResultStatus == cloudapi.ResultStatusFailed {
//nolint:golint
Expand Down
4 changes: 3 additions & 1 deletion cmd/login_cloud.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"os"
"syscall"

"github.com/fatih/color"
"github.com/sirupsen/logrus"
"github.com/spf13/afero"
"github.com/spf13/cobra"
Expand Down Expand Up @@ -141,7 +142,8 @@ This will set the default token used when just "k6 run -o cloud" is passed.`,
}

if newCloudConf.Token.Valid {
fprintf(stdout, " token: %s\n", ui.ValueColor.Sprint(newCloudConf.Token.String))
valueColor := getColor(noColor || !stdoutTTY, color.FgCyan)
fprintf(stdout, " token: %s\n", valueColor.Sprint(newCloudConf.Token.String))
}
return nil
},
Expand Down
4 changes: 1 addition & 3 deletions cmd/pause.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (

v1 "go.k6.io/k6/api/v1"
"go.k6.io/k6/api/v1/client"
"go.k6.io/k6/ui"
)

func getPauseCmd(ctx context.Context) *cobra.Command {
Expand All @@ -50,8 +49,7 @@ func getPauseCmd(ctx context.Context) *cobra.Command {
if err != nil {
return err
}
ui.Dump(stdout, status)
return nil
return yamlPrint(stdout, status)
},
}
return pauseCmd
Expand Down
5 changes: 2 additions & 3 deletions cmd/resume.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (

v1 "go.k6.io/k6/api/v1"
"go.k6.io/k6/api/v1/client"
"go.k6.io/k6/ui"
)

func getResumeCmd(ctx context.Context) *cobra.Command {
Expand All @@ -50,8 +49,8 @@ func getResumeCmd(ctx context.Context) *cobra.Command {
if err != nil {
return err
}
ui.Dump(stdout, status)
return nil

return yamlPrint(stdout, status)
},
}
return resumeCmd
Expand Down
39 changes: 12 additions & 27 deletions cmd/root.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import (
"sync"
"time"

"github.com/fatih/color"
"github.com/mattn/go-colorable"
"github.com/mattn/go-isatty"
"github.com/sirupsen/logrus"
Expand All @@ -43,16 +42,15 @@ import (
"go.k6.io/k6/log"
)

var BannerColor = color.New(color.FgCyan)

//TODO: remove these global variables
//nolint:gochecknoglobals
var (
outMutex = &sync.Mutex{}
stdoutTTY = isatty.IsTerminal(os.Stdout.Fd()) || isatty.IsCygwinTerminal(os.Stdout.Fd())
stderrTTY = isatty.IsTerminal(os.Stderr.Fd()) || isatty.IsCygwinTerminal(os.Stderr.Fd())
stdout = &consoleWriter{colorable.NewColorableStdout(), stdoutTTY, outMutex, nil}
stderr = &consoleWriter{colorable.NewColorableStderr(), stderrTTY, outMutex, nil}
outMutex = &sync.Mutex{}
isDumbTerm = os.Getenv("TERM") == "dumb"
stdoutTTY = !isDumbTerm && (isatty.IsTerminal(os.Stdout.Fd()) || isatty.IsCygwinTerminal(os.Stdout.Fd()))
stderrTTY = !isDumbTerm && (isatty.IsTerminal(os.Stderr.Fd()) || isatty.IsCygwinTerminal(os.Stderr.Fd()))
stdout = &consoleWriter{colorable.NewColorableStdout(), stdoutTTY, outMutex, nil}
stderr = &consoleWriter{colorable.NewColorableStderr(), stderrTTY, outMutex, nil}
)

const (
Expand Down Expand Up @@ -97,7 +95,7 @@ func newRootCommand(ctx context.Context, logger *logrus.Logger, fallbackLogger l
c.cmd = &cobra.Command{
Use: "k6",
Short: "a next-generation load generator",
Long: BannerColor.Sprintf("\n%s", consts.Banner()),
Long: "\n" + getBanner(noColor || !stdoutTTY),
SilenceUsage: true,
SilenceErrors: true,
PersistentPreRunE: c.persistentPreRunE,
Expand Down Expand Up @@ -136,22 +134,6 @@ func (c *rootCommand) persistentPreRunE(cmd *cobra.Command, args []string) error
c.loggerIsRemote = true
}

if noColor {
// TODO: figure out something else... currently, with the wrappers
// below, we're stripping any colors from the output after we've
// added them. The problem is that, besides being very inefficient,
// this actually also strips other special characters from the
// intended output, like the progressbar formatting ones, which
// would otherwise be fine (in a TTY).
//
// It would be much better if we avoid messing with the output and
// instead have a parametrized instance of the color library. It
// will return colored output if colors are enabled and simply
// return the passed input as-is (i.e. be a noop) if colors are
// disabled...
stdout.Writer = colorable.NewNonColorable(os.Stdout)
stderr.Writer = colorable.NewNonColorable(os.Stderr)
}
stdlog.SetOutput(c.logger.Writer())
c.logger.Debugf("k6 version: v%s", consts.FullVersion())
return nil
Expand Down Expand Up @@ -276,10 +258,14 @@ func (c *rootCommand) setupLoggers() (<-chan struct{}, error) {
if c.verbose {
c.logger.SetLevel(logrus.DebugLevel)
}

loggerForceColors := false // disable color by default
switch c.logOutput {
case "stderr":
loggerForceColors = !noColor && stderrTTY
c.logger.SetOutput(stderr)
case "stdout":
loggerForceColors = !noColor && stdoutTTY
c.logger.SetOutput(stdout)
case "none":
c.logger.SetOutput(ioutil.Discard)
Expand All @@ -295,7 +281,6 @@ func (c *rootCommand) setupLoggers() (<-chan struct{}, error) {
c.logger.AddHook(hook)
c.logger.SetOutput(ioutil.Discard) // don't output to anywhere else
c.logFmt = "raw"
noColor = true // disable color
}

switch c.logFmt {
Expand All @@ -306,7 +291,7 @@ func (c *rootCommand) setupLoggers() (<-chan struct{}, error) {
c.logger.SetFormatter(&logrus.JSONFormatter{})
c.logger.Debug("Logger format: JSON")
default:
c.logger.SetFormatter(&logrus.TextFormatter{ForceColors: stderrTTY, DisableColors: noColor})
c.logger.SetFormatter(&logrus.TextFormatter{ForceColors: loggerForceColors, DisableColors: noColor})
c.logger.Debug("Logger format: TEXT")
}
return ch, nil
Expand Down
9 changes: 7 additions & 2 deletions cmd/run.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ a commandline interface for interacting with it.`,
Args: exactArgsWithMsg(1, "arg should either be \"-\", if reading script from stdin, or a path to a script file"),
RunE: func(cmd *cobra.Command, args []string) error {
// TODO: disable in quiet mode?
_, _ = BannerColor.Fprintf(stdout, "\n%s\n\n", consts.Banner())
_, _ = fmt.Fprintf(stdout, "\n%s\n\n", getBanner(noColor || !stdoutTTY))

logger.Debug("Initializing the runner...")

Expand Down Expand Up @@ -233,7 +233,7 @@ a commandline interface for interacting with it.`,

printExecutionDescription(
"local", filename, "", conf, execScheduler.GetState().ExecutionTuple,
executionPlan, outputs)
executionPlan, outputs, noColor || !stdoutTTY)

// Trap Interrupts, SIGINTs and SIGTERMs.
sigC := make(chan os.Signal, 1)
Expand Down Expand Up @@ -299,6 +299,11 @@ a commandline interface for interacting with it.`,
Metrics: engine.Metrics,
RootGroup: engine.ExecutionScheduler.GetRunner().GetDefaultGroup(),
TestRunDuration: executionState.GetCurrentTestRunDuration(),
NoColor: noColor,
UIState: lib.UIState{
IsStdOutTTY: stdoutTTY,
IsStdErrTTY: stderrTTY,
},
})
if err == nil {
err = handleSummaryResult(afero.NewOsFs(), stdout, stderr, summaryResult)
Expand Down
5 changes: 2 additions & 3 deletions cmd/scale.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (

v1 "go.k6.io/k6/api/v1"
"go.k6.io/k6/api/v1/client"
"go.k6.io/k6/ui"
)

func getScaleCmd(ctx context.Context) *cobra.Command {
Expand All @@ -54,8 +53,8 @@ func getScaleCmd(ctx context.Context) *cobra.Command {
if err != nil {
return err
}
ui.Dump(stdout, status)
return nil

return yamlPrint(stdout, status)
},
}

Expand Down
5 changes: 2 additions & 3 deletions cmd/stats.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
"github.com/spf13/cobra"

"go.k6.io/k6/api/v1/client"
"go.k6.io/k6/ui"
)

func getStatsCmd(ctx context.Context) *cobra.Command {
Expand All @@ -46,8 +45,8 @@ func getStatsCmd(ctx context.Context) *cobra.Command {
if err != nil {
return err
}
ui.Dump(stdout, metrics)
return nil

return yamlPrint(stdout, metrics)
},
}
return statsCmd
Expand Down
5 changes: 2 additions & 3 deletions cmd/status.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
"github.com/spf13/cobra"

"go.k6.io/k6/api/v1/client"
"go.k6.io/k6/ui"
)

func getStatusCmd(ctx context.Context) *cobra.Command {
Expand All @@ -46,8 +45,8 @@ func getStatusCmd(ctx context.Context) *cobra.Command {
if err != nil {
return err
}
ui.Dump(stdout, status)
return nil

return yamlPrint(stdout, status)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this is a good case for wrapping errors as "could not marshal YAML:" will be quite strange error for someone calling k6 pause or co.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, though to be fair, the new state isn't any worse than the previous behavior where it would have printed just the raw YAML marshaling error, without even the could not marshal YAML: in front of it 😅 but yeah, I may wrap these errors locally as well...

},
}
return statusCmd
Expand Down
Loading