Skip to content
This repository has been archived by the owner on Jan 30, 2025. It is now read-only.

Fix/linter errs test browser #245

Merged
merged 2 commits into from
Feb 18, 2022
Merged

Fix/linter errs test browser #245

merged 2 commits into from
Feb 18, 2022

Conversation

inancgumus
Copy link
Member

@inancgumus inancgumus commented Feb 18, 2022

This improvement PR fixes the linter errors in the test browser.

It also starts using t.Helper and t.Fatalf instead of panicking in the test. Benefits: Easily pinpoint the problematic code line in the tests and test browser with a leaner fatal message.

Example:

kbtf -run='TestPageSetExtraHTTPHeaders'
--- FAIL: TestPageSetExtraHTTPHeaders (0.30s)
    page_test.go:96: You should enable HTTP test server, see: withHTTPServer option

Instead of:

kbtf -run='TestPageSetExtraHTTPHeaders'
--- FAIL: TestPageSetExtraHTTPHeaders (0.32s)
panic: You should enable HTTP test server, see: withHTTPServer option [recovered]
	panic: You should enable HTTP test server, see: withHTTPServer option

goroutine 7 [running]:
testing.tRunner.func1.2({0x105189f00, 0x105333f10})
	/usr/local/go/src/testing/testing.go:1209 +0x258
testing.tRunner.func1(0x140005091e0)
	/usr/local/go/src/testing/testing.go:1212 +0x284
panic({0x105189f00, 0x105333f10})
	/usr/local/go/src/runtime/panic.go:1038 +0x21c

# .... hundreds of lines more

Related: grafana/k6#4496

@inancgumus inancgumus requested a review from imiric February 18, 2022 08:23
@inancgumus inancgumus self-assigned this Feb 18, 2022
@imiric imiric mentioned this pull request Jan 30, 2025
60 tasks
Copy link
Contributor

@imiric imiric left a comment

Choose a reason for hiding this comment

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

👍

@inancgumus inancgumus merged commit 81bac67 into main Feb 18, 2022
@inancgumus inancgumus deleted the fix/linter-errs-test-browser branch February 18, 2022 10:53
@imiric imiric added this to the v0.2.0 milestone Feb 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants