Skip to content

Conversation

jonathanbeber
Copy link
Contributor

Change error computation on JSON Unmarshall

The Unmarshall function on the encoding/JSON default library returns
different errors for different go versions. On Go 1.12, the version used
currently on the CI system it returns json: cannot unmarshal number into Go struct field PostgresSpec.teamId of type string. On Go 1.13.5 it
returns json: cannot unmarshal number into Go struct field PostgresSpec.spec.teamId of type string. The new version includes more
details of the whole structure being unmarshalled.

This commit introduces the same error but one level deeper on the JSON
structure. It creates consistency across different Go versions.


Create subtests on table test scenarios

The Run method of T allows defining subtests creating hierarchical tests.
It provides better visibility of tests in case of failure. More
details on https://golang.org/pkg/testing/.

This commit converts each test scenario on
pkg/apis/acid.zalan.do/v1/util_test.go to subtests, providing a better
visibility and the debugging environment when working with tests. The
following code snippet shows a error during test execution with
subtests:

--- FAIL: TestUnmarshalMaintenanceWindow (0.00s)
    --- FAIL: TestUnmarshalMaintenanceWindow/expect_error_as_'From'_is_later_than_'To' (0.00s)

It included a about field on test scenarios describing the test
purpose and/or it expected output. When a description was provided with
comments it was moved to the about field.

The [Unmarshall function][1] on the encoding/JSON default library returns
different errors for different go versions. On Go 1.12, the version used
currently on the CI system it returns `json: cannot unmarshal number into
Go struct field PostgresSpec.teamId of type string`. On Go 1.13.5 it
returns `json: cannot unmarshal number into Go struct field
PostgresSpec.spec.teamId of type string`. The new version includes more
details of the whole structure being unmarshelled.

This commit introduces the same error but one level deeper on the JSON
structure. It creates consistency across different Go versions.

[1]: https://godoc.org/encoding/json#Unmarshal
@jonathanbeber
Copy link
Contributor Author

For reviewers, I recommend the diff view without empty spaces

@jonathanbeber
Copy link
Contributor Author

On CI it has a better test description as well:

--- PASS: TestClusterName (0.00s)
    --- PASS: TestClusterName/common_team_and_cluster_name (0.00s)
    --- PASS: TestClusterName/cluster_name_with_hyphen (0.00s)
    --- PASS: TestClusterName/cluster_and_team_name_with_hyphen (0.00s)
    --- PASS: TestClusterName/expect_error_as_cluster_name_is_just_hyphens (0.00s)
    --- PASS: TestClusterName/expect_error_as_cluster_name_is_too_long (0.00s)
    --- PASS: TestClusterName/expect_error_as_cluster_name_does_not_match_{TEAM}-{NAME}_format (0.00s)
    --- PASS: TestClusterName/expect_error_as_team_and_cluster_name_are_empty (0.00s)
    --- PASS: TestClusterName/expect_error_as_cluster_name_is_empty_and_team_name_is_a_hyphen (0.00s)
    --- PASS: TestClusterName/expect_error_as_cluster_name_is_empty,_team_name_is_a_hyphen_and_cluster_name_is_empty (0.00s)
    --- PASS: TestClusterName/expect_error_as_cluster_and_team_name_are_hyphens (0.00s)

The Run method of T allows defining subtests creating hierarchical tests.
It provides better visibility of tests in case of failure. More
details on https://golang.org/pkg/testing/.

This commit converts each test scenario on
pkg/apis/acid.zalan.do/v1/util_test.go to subtests, providing a better
visibility and the debugging environment when working with tests. The
following code snippet shows an error during test execution with
subtests:

```
--- FAIL: TestUnmarshalMaintenanceWindow (0.00s)
    --- FAIL: TestUnmarshalMaintenanceWindow/expect_error_as_'From'_is_later_than_'To' (0.00s)
```

It included a `about` field on test scenarios describing the test
purpose and/or it expected output. When a description was provided with
comments it was moved to the about field.
@FxKu
Copy link
Member

FxKu commented Jan 27, 2020

Thanks for this contribution. Works also with my Go 1.12 so 👍
At some point we should make our other tests also more wordy.

@FxKu
Copy link
Member

FxKu commented Jan 27, 2020

👍

1 similar comment
@Jan-M
Copy link
Member

Jan-M commented Jan 27, 2020

👍

@Jan-M Jan-M merged commit fddaf0f into zalando:master Jan 27, 2020
@jonathanbeber jonathanbeber deleted the subtest-v1 branch January 27, 2020 14:00
@FxKu FxKu added this to the 1.4 milestone Jan 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants