Skip to content

Commit

Permalink
refactor: remove ndpayload and lenpayload
Browse files Browse the repository at this point in the history
Those output formats are undocumented and seem to be only used in tests.
This change removes their implementation and replaces it with error
message to use JSON instead.

I also refactored tests to test the --enc=json response format instead
of imaginary one, making tests more useful as they also act as
regression tests for HTTP RPC.
  • Loading branch information
lidel committed Oct 6, 2021
1 parent ba52fd4 commit c74bbe7
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 33 deletions.
22 changes: 6 additions & 16 deletions core/commands/pubsub.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package commands

import (
"context"
"encoding/binary"
"fmt"
"io"
"io/ioutil"
Expand Down Expand Up @@ -127,24 +126,15 @@ TOPIC ENCODING
_, err = w.Write(dec)
return err
}),
// --enc=ndpayload is not documented, byt used internally by sharness tests
// DEPRECATED, undocumented format we used in tests, but not anymore
// <message.payload>\n<message.payload>\n
"ndpayload": cmds.MakeTypedEncoder(func(req *cmds.Request, w io.Writer, psm *pubsubMessage) error {
_, dec, err := mbase.Decode(psm.Data)
if err != nil {
return err
}
data := append(dec, '\n')
_, err = w.Write(data)
return err
return errors.New("--enc=ndpayload was removed, use --enc=json instead")
}),
// ¯\_(ツ)_/¯ – seems unused, can we remove this?
// DEPRECATED, uncodumented format we used in tests, but not anymore
// <varint-len><message.payload><varint-len><message.payload>
"lenpayload": cmds.MakeTypedEncoder(func(req *cmds.Request, w io.Writer, psm *pubsubMessage) error {
buf := make([]byte, 8, len(psm.Data)+8)

n := binary.PutUvarint(buf, uint64(len(psm.Data)))
buf = append(buf[:n], psm.Data...)
_, err := w.Write(buf)
return err
return errors.New("--enc=lenpayload was removed, use --enc=json instead")
}),
},
Type: pubsubMessage{},
Expand Down
15 changes: 7 additions & 8 deletions test/sharness/t0180-pubsub-gossipsub.sh
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,16 @@ test_expect_success 'peer ids' '
'

test_expect_success 'pubsub' '
echo "testOK" > expected &&
echo -n "testOK" | ipfs multibase encode -b base64url > expected &&
touch empty &&
mkfifo wait ||
test_fsh echo init fail
# ipfs pubsub sub is long-running so we need to start it in the background and
# wait put its output somewhere where we can access it
(
ipfsi 0 pubsub sub --enc=ndpayload testTopic | if read line; then
echo $line > actual &&
ipfsi 0 pubsub sub --enc=json testTopic | if read line; then
echo $line | jq -j .data > actual &&
echo > wait
fi
) &
Expand Down Expand Up @@ -64,15 +64,15 @@ test_expect_success "wait until echo > wait executed" '
'

test_expect_success "wait for another pubsub message" '
echo "testOK2" > expected &&
echo -n "testOK2" | ipfs multibase encode -b base64url > expected &&
mkfifo wait2 ||
test_fsh echo init fail
# ipfs pubsub sub is long-running so we need to start it in the background and
# wait put its output somewhere where we can access it
(
ipfsi 2 pubsub sub --enc=ndpayload testTopic | if read line; then
echo $line > actual &&
ipfsi 2 pubsub sub --enc=json testTopic | if read line; then
echo $line | jq -j .data > actual &&
echo > wait2
fi
) &
Expand All @@ -83,11 +83,10 @@ test_expect_success "wait until ipfs pubsub sub is ready to do work" '
'

test_expect_success "publish something" '
echo "testOK2" | ipfsi 1 pubsub pub testTopic &> pubErr
echo -n "testOK2" | ipfsi 1 pubsub pub testTopic &> pubErr
'

test_expect_success "wait until echo > wait executed" '
echo "testOK2" > expected &&
cat wait2 &&
test_cmp pubErr empty &&
test_cmp expected actual
Expand Down
17 changes: 8 additions & 9 deletions test/sharness/t0180-pubsub.sh
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,16 @@ run_pubsub_tests() {

# ipfs pubsub sub
test_expect_success 'pubsub' '
echo "testOK" > expected &&
echo -n "testOK" | ipfs multibase encode -b base64url > expected &&
touch empty &&
mkfifo wait ||
test_fsh echo init fail
# ipfs pubsub sub is long-running so we need to start it in the background and
# wait put its output somewhere where we can access it
(
ipfsi 0 pubsub sub --enc=ndpayload testTopic | if read line; then
echo $line > actual &&
ipfsi 0 pubsub sub --enc=json testTopic | if read line; then
echo $line | jq -j .data > actual &&
echo > wait
fi
) &
Expand Down Expand Up @@ -61,15 +61,15 @@ run_pubsub_tests() {
'

test_expect_success "wait for another pubsub message" '
echo "testOK2" > expected &&
echo -n "testOK2" | ipfs multibase encode -b base64url > expected &&
mkfifo wait2 ||
test_fsh echo init fail
# ipfs pubsub sub is long-running so we need to start it in the background and
# wait put its output somewhere where we can access it
(
ipfsi 2 pubsub sub --enc=ndpayload testTopic | if read line; then
echo $line > actual &&
ipfsi 2 pubsub sub --enc=json testTopic | if read line; then
echo $line | jq -j .data > actual &&
echo > wait2
fi
) &
Expand All @@ -80,11 +80,10 @@ run_pubsub_tests() {
'

test_expect_success "publish something" '
echo "testOK2" | ipfsi 3 pubsub pub testTopic &> pubErr
echo -n "testOK2" | ipfsi 3 pubsub pub testTopic &> pubErr
'

test_expect_success "wait until echo > wait executed" '
echo "testOK2" > expected &&
cat wait2 &&
test_cmp pubErr empty &&
test_cmp expected actual
Expand Down Expand Up @@ -114,7 +113,7 @@ startup_cluster $NUM_NODES --enable-pubsub-experiment

test_expect_success 'set node 4 to listen on testTopic' '
rm -f node4_actual &&
ipfsi 4 pubsub sub --enc=ndpayload testTopic > node4_actual &
ipfsi 4 pubsub sub --enc=json testTopic > node4_actual &
'

run_pubsub_tests
Expand Down

0 comments on commit c74bbe7

Please sign in to comment.