Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add command to generate cryptosign challenge #1

Merged

Conversation

muzzammilshahid
Copy link
Member

No description provided.

@muzzammilshahid muzzammilshahid requested a review from om26er June 2, 2024 08:27
@muzzammilshahid muzzammilshahid force-pushed the generate-cryptosign-challenge branch 5 times, most recently from ad41d82 to 550c8a1 Compare June 3, 2024 10:57
@@ -0,0 +1,16 @@
package main
Copy link
Member

Choose a reason for hiding this comment

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

should be moved out of the cmd package to the root of this repository. Create helpers.go at the root of the project

@muzzammilshahid muzzammilshahid force-pushed the generate-cryptosign-challenge branch from 550c8a1 to 8414184 Compare June 3, 2024 12:38
@muzzammilshahid muzzammilshahid requested a review from om26er June 3, 2024 12:38
@muzzammilshahid muzzammilshahid force-pushed the generate-cryptosign-challenge branch from 8414184 to acc5548 Compare June 3, 2024 12:45
helpers.go Outdated
@@ -0,0 +1,16 @@
package wampproto_cli
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
package wampproto_cli
package wampprotocli

helpers.go Outdated
"encoding/hex"
)

func HexToBase64String(hexStr string) (string, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
func HexToBase64String(hexStr string) (string, error) {
func HexToBase64(hexStr string) (string, error) {

helpers_test.go Outdated

"github.com/stretchr/testify/require"

main "github.com/xconnio/wampproto-cli"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
main "github.com/xconnio/wampproto-cli"
main "github.com/xconnio/wampproto-cli"

use the package name, not main

hello := messages.NewHello("realm1", "test", nil, nil, []string{"anonymous"})
serializer := &serializers.MsgPackSerializer{}
data, err := serializer.Serialize(hello)
const versionString = "0.1.0"
Copy link
Member

Choose a reason for hiding this comment

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

move inside the below const stanza

return c, nil
}

func Run(args []string) error {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
func Run(args []string) error {
func Run(args []string) error {

This function may just return the output string and the error. Then it becomes super easy to work in tests as well. No hacks needed to check shell output

@muzzammilshahid muzzammilshahid force-pushed the generate-cryptosign-challenge branch from acc5548 to 6ef4708 Compare June 4, 2024 10:33
@muzzammilshahid muzzammilshahid force-pushed the generate-cryptosign-challenge branch from 6ef4708 to 9aeae3b Compare June 4, 2024 10:34
@muzzammilshahid muzzammilshahid requested a review from om26er June 4, 2024 10:41
main "github.com/xconnio/wampproto-cli/cmd/wampproto"
)

func TestRunGenerateChallenge(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

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

These tests can be simplified, no need to loop, no need to check regex. If the output is supposed to be hex, just decode and if it works its correct. Same for base64.

helpers_test.go Outdated
}

// test invalid HexStrings
for _, rawStr := range []string{
Copy link
Member

Choose a reason for hiding this comment

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

use t.Run() and make them named tests within.

@muzzammilshahid muzzammilshahid force-pushed the generate-cryptosign-challenge branch 4 times, most recently from cf8930a to 2a5bd90 Compare June 4, 2024 11:44
@muzzammilshahid muzzammilshahid force-pushed the generate-cryptosign-challenge branch from 2a5bd90 to b9e9b9e Compare June 4, 2024 11:45
@muzzammilshahid muzzammilshahid merged commit 0006388 into xconnio:main Jun 4, 2024
1 check passed
@muzzammilshahid muzzammilshahid deleted the generate-cryptosign-challenge branch June 4, 2024 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants