All test stuff #88

i-norden · 2023-01-25T21:54:53Z

ABastionOfSanity commented

2023-01-25 21:54:53 +00:00

(Migrated from github.com)

branch for testing that includes CID generation fix and workflow sdk_tests changes.

❤️ 1

codecov-commenter commented

2023-01-25 22:45:45 +00:00

(Migrated from github.com)

Codecov Report

Merging #88 (fdfe6ca) into main (ab2ea51) will decrease coverage by 0.19%.
The diff coverage is 66.66%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #88      +/-   ##
==========================================
- Coverage   66.57%   66.38%   -0.19%     
==========================================
  Files         145      148       +3     
  Lines       15953    16054     +101     
==========================================
+ Hits        10620    10657      +37     
- Misses       4859     4920      +61     
- Partials      474      477       +3

Impacted Files	Coverage Δ
utils/json.go	`58.73% <62.50%> (ø)`
x/registry/keeper/keeper.go	`48.92% <100.00%> (ø)`
utils/types.go	`0.00% <0.00%> (ø)`
utils/mnemonic.go	`0.00% <0.00%> (ø)`

# [Codecov](https://codecov.io/gh/cerc-io/laconicd/pull/88?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) Report > Merging [#88](https://codecov.io/gh/cerc-io/laconicd/pull/88?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (fdfe6ca) into [main](https://codecov.io/gh/cerc-io/laconicd/commit/ab2ea51aac6e3c26a41dd4028d8d5d8884e13b50?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (ab2ea51) will **decrease** coverage by `0.19%`. > The diff coverage is `66.66%`. :mega: This organization is not using Codecov’s [GitHub App Integration](https://github.com/apps/codecov). We recommend you install it so Codecov can continue to function properly for your repositories. [Learn more](https://about.codecov.io/blog/codecov-is-updating-its-github-integration/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) <details><summary>Additional details and impacted files</summary> [![Impacted file tree graph](https://codecov.io/gh/cerc-io/laconicd/pull/88/graphs/tree.svg?width=650&height=150&src=pr&token=5tWmn7LxfO&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)](https://codecov.io/gh/cerc-io/laconicd/pull/88?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) ```diff @@ Coverage Diff @@ ## main #88 +/- ## ========================================== - Coverage 66.57% 66.38% -0.19% ========================================== Files 145 148 +3 Lines 15953 16054 +101 ========================================== + Hits 10620 10657 +37 - Misses 4859 4920 +61 - Partials 474 477 +3 ``` | [Impacted Files](https://codecov.io/gh/cerc-io/laconicd/pull/88?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | | |---|---|---| | [utils/json.go](https://codecov.io/gh/cerc-io/laconicd/pull/88?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-dXRpbHMvanNvbi5nbw==) | `58.73% <62.50%> (ø)` | | | [x/registry/keeper/keeper.go](https://codecov.io/gh/cerc-io/laconicd/pull/88?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-eC9yZWdpc3RyeS9rZWVwZXIva2VlcGVyLmdv) | `48.92% <100.00%> (ø)` | | | [utils/types.go](https://codecov.io/gh/cerc-io/laconicd/pull/88?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-dXRpbHMvdHlwZXMuZ28=) | `0.00% <0.00%> (ø)` | | | [utils/mnemonic.go](https://codecov.io/gh/cerc-io/laconicd/pull/88?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-dXRpbHMvbW5lbW9uaWMuZ28=) | `0.00% <0.00%> (ø)` | | </details>

github-code-scanning[bot] (Migrated from github.com) reviewed 2023-01-26 21:37:12 +00:00

gql/server.go

github-code-scanning[bot] (Migrated from github.com) commented

2023-01-26 21:37:11 +00:00

Found an HTTP server without TLS. Use 'http.ListenAndServeTLS' instead. See https://golang.org/pkg/net/http/#ListenAndServeTLS for more information.

Show more details

## Found an HTTP server without TLS. Use 'http.ListenAndServeTLS' instead. See https://golang.org/pkg/net/http/#ListenAndServeTLS for more information. Found an HTTP server without TLS. Use 'http.ListenAndServeTLS' instead. See https://golang.org/pkg/net/http/#ListenAndServeTLS for more information. [Show more details](https://github.com/cerc-io/laconicd/security/code-scanning/600)

github-code-scanning[bot] (Migrated from github.com) reviewed 2023-01-26 22:00:52 +00:00

gql/server.go

github-code-scanning[bot] (Migrated from github.com) commented

2023-01-26 22:00:51 +00:00

Found an HTTP server without TLS. Use 'http.ListenAndServeTLS' instead. See https://golang.org/pkg/net/http/#ListenAndServeTLS for more information.

Show more details

## Found an HTTP server without TLS. Use 'http.ListenAndServeTLS' instead. See https://golang.org/pkg/net/http/#ListenAndServeTLS for more information. Found an HTTP server without TLS. Use 'http.ListenAndServeTLS' instead. See https://golang.org/pkg/net/http/#ListenAndServeTLS for more information. [Show more details](https://github.com/cerc-io/laconicd/security/code-scanning/601)

i-norden reviewed 2023-01-27 19:01:26 +00:00

i-norden left a comment

Just one requested modification for the new unit test, otherwise LGTM!

Dockerfile

i-norden commented

2023-01-27 18:54:10 +00:00

SUPER nitpicky: remove space at eol

utils/json_test.go

						
				@ -0,0 +26,4 @@

					}

					for _, tc := range testCases {

						deprecatedAndCorrect, _ := CIDFromJSONBytes([]byte(tc.content))

i-norden commented

2023-01-27 18:54:42 +00:00

We should either require.NoError() the errors returned by these functions or make sure the returned result isn't nil/empty, because if both functions were to error out and return a nil/empty result it would evaluate them as equal and the test would pass.

We should either `require.NoError()` the errors returned by these functions or make sure the returned result isn't nil/empty, because if both functions were to error out and return a nil/empty result it would evaluate them as equal and the test would pass.

i-norden reviewed 2023-01-27 19:04:58 +00:00

utils/json_test.go

						
				@ -0,0 +26,4 @@

					}

					for _, tc := range testCases {

						deprecatedAndCorrect, _ := CIDFromJSONBytes([]byte(tc.content))

i-norden commented

2023-01-27 19:04:57 +00:00

Also, ideally we'd have a static fixture for what the expected CID is for some input content- so that if both functions are updated in the same change set we can be certain we didn't break them but in the same way such that this test still passes.

But, since we will be getting rid of CIDFromJSONBytes soon we shouldn't do that here. Once we get rid of CIDFromJSONBytes, we will have to test against a static fixture.

Also, ideally we'd have a static fixture for what the expected CID is for some input content- so that if both functions are updated in the same change set we can be certain we didn't break them but in the same way such that this test still passes. But, since we will be getting rid of `CIDFromJSONBytes` soon we shouldn't do that here. Once we get rid of `CIDFromJSONBytes`, we will have to test against a static fixture.

Sign in to join this conversation.

No reviewers