Add plumbing between spanner & http for missing one impl feature list#1277
Add plumbing between spanner & http for missing one impl feature list#1277
Conversation
KyleJu
changed the title
jcscottiii
left a comment
jcscottiii
left a comment
There was a problem hiding this comment.
LGTM with a suggestion to use the valuePtr helper. This will prevent us from accidentally using == for pointer comparisons (which only checks addresses) and encourage the use of reflect.DeepEqual for reliable value comparisons.
backend/pkg/httpserver/list_missing_one_implementation_features_test.go
Outdated
Show resolved
Hide resolved
jcscottiii
left a comment
jcscottiii
left a comment
There was a problem hiding this comment.
Apologies for the last minute change. You should send the raw requests and check the responses. The other tests for backend/pkg/httpserver do that now.
Also, you are missing the 400 case for the ErrInvalidPageToken.
Take a look at this file for an example of both: https://github.com/GoogleChrome/webstatus.dev/blob/dd082e1045062e143754509026c44b53cf12180c/backend/pkg/httpserver/list_chromium_usage_test.go
jcscottiii
dismissed
their stale review
Apologies. I missed that the tests should send raw requests like others. You'll see that the test code in #904 has been updated to follow that pattern: https://github.com/GoogleChrome/webstatus.dev/blob/main/backend/pkg/httpserver/list_aggregated_feature_support_test.go
2 participants
#1181. This change adds the spanner adapter conversion layer for the missing one implementation feature list endpoint. This change is drawn from #904