search

elastic / go-ucfg

Golang universal configuration
Apache License 2.0
82 stars 48 forks source link

Don't panic and fail test early #156

Open urso opened 4 years ago

urso commented 4 years ago

The tests in go-ucfg use the .../testify/assert package for validation, but sometimes also for checking on intermediate errors. The assert package only logs an error to *testing.T, but continues the test run. If there is a dependency on an operation to succeed (e.g. calling NewFrom), then the test should fail immediately pointing out the failing line in code. This can be achieved by using t.Fatal(f), use .../testify/require instead of assert, or use constructors that can panic like MustNewFrom instead of NewFrom.

By not failing and returning the test early, there might be a chance that validation/operations will panic. In this case the errors/logs stored in *testing.T will not be written to console, and the actual failure can't be inspected.

alrs commented 4 years ago

@urso I've jumped in to this.

Is https://github.com/davecgh/go-spew approved for elastic projects? It makes for much more readable test errors.

urso commented 4 years ago

go-spew is fine.

urso commented 3 years ago

Note: testify package is basically everywhere. Not just in go-ucfg, but it is also used by many other projects. For sake of consistency we should switch to require.X instead of trying to get rid or replace testify.