Re: [PATCH v3 11/35] test-pkt-line: introduce a packet-line test helper

[Brandon Williams]

On 03/02, Jeff King wrote:
> On Fri, Feb 23, 2018 at 01:22:31PM -0800, Brandon Williams wrote:
> 
> > On 02/22, Stefan Beller wrote:
> > > On Tue, Feb 6, 2018 at 5:12 PM, Brandon Williams  
> > > wrote:
> > > 
> > > > +static void pack_line(const char *line)
> > > > +{
> > > > +   if (!strcmp(line, "") || !strcmp(line, "\n"))
> > > 
> > > From our in-office discussion:
> > > v1/v0 packs pktlines twice in http, which is not possible to
> > > construct using this test helper when using the same string
> > > for the packed and unpacked representation of flush and delim packets,
> > > i.e. test-pkt-line --pack $(test-pkt-line --pack ) would produce
> > > '' instead of '0009\n'.
> > > To fix it we'd have to replace the unpacked versions of these pkts to
> > > something else such as "FLUSH" "DELIM".
> > > 
> > > However as we do not anticipate the test helper to be used in further
> > > tests for v0, this ought to be no big issue.
> > > Maybe someone else cares though?
> > 
> > I'm going to punt and say, if someone cares enough they can update this
> > test-helper when they want to use it for v1/v0 stuff.
> 
> I recently add packetize and depacketize helpers for testing v0 streams;
> see 4414a15002 (t/lib-git-daemon: add network-protocol helpers,
> 2018-01-24). Is it worth folding these together?

I didn't know something like that existed! (of course if it was just
added this year then it didn't exist when I started working on this
stuff).  Yeah its probably a good idea to fold these together, I can
take a look at how your packetize and depacketize helpers work and add
the small amount of functionality that I'd need to replace the helper I
made.

> 
> -Peff

-- 
Brandon Williams

Re: [PATCH v3 11/35] test-pkt-line: introduce a packet-line test helper

[Jeff King]

On Fri, Feb 23, 2018 at 01:22:31PM -0800, Brandon Williams wrote:

> On 02/22, Stefan Beller wrote:
> > On Tue, Feb 6, 2018 at 5:12 PM, Brandon Williams  wrote:
> > 
> > > +static void pack_line(const char *line)
> > > +{
> > > +   if (!strcmp(line, "") || !strcmp(line, "\n"))
> > 
> > From our in-office discussion:
> > v1/v0 packs pktlines twice in http, which is not possible to
> > construct using this test helper when using the same string
> > for the packed and unpacked representation of flush and delim packets,
> > i.e. test-pkt-line --pack $(test-pkt-line --pack ) would produce
> > '' instead of '0009\n'.
> > To fix it we'd have to replace the unpacked versions of these pkts to
> > something else such as "FLUSH" "DELIM".
> > 
> > However as we do not anticipate the test helper to be used in further
> > tests for v0, this ought to be no big issue.
> > Maybe someone else cares though?
> 
> I'm going to punt and say, if someone cares enough they can update this
> test-helper when they want to use it for v1/v0 stuff.

I recently add packetize and depacketize helpers for testing v0 streams;
see 4414a15002 (t/lib-git-daemon: add network-protocol helpers,
2018-01-24). Is it worth folding these together?

-Peff

Re: [PATCH v3 11/35] test-pkt-line: introduce a packet-line test helper

[Brandon Williams]

On 02/22, Stefan Beller wrote:
> On Tue, Feb 6, 2018 at 5:12 PM, Brandon Williams  wrote:
> 
> > +static void pack_line(const char *line)
> > +{
> > +   if (!strcmp(line, "") || !strcmp(line, "\n"))
> 
> From our in-office discussion:
> v1/v0 packs pktlines twice in http, which is not possible to
> construct using this test helper when using the same string
> for the packed and unpacked representation of flush and delim packets,
> i.e. test-pkt-line --pack $(test-pkt-line --pack ) would produce
> '' instead of '0009\n'.
> To fix it we'd have to replace the unpacked versions of these pkts to
> something else such as "FLUSH" "DELIM".
> 
> However as we do not anticipate the test helper to be used in further
> tests for v0, this ought to be no big issue.
> Maybe someone else cares though?

I'm going to punt and say, if someone cares enough they can update this
test-helper when they want to use it for v1/v0 stuff.

-- 
Brandon Williams

Re: [PATCH v3 11/35] test-pkt-line: introduce a packet-line test helper

[Stefan Beller]

On Tue, Feb 6, 2018 at 5:12 PM, Brandon Williams  wrote:

> +static void pack_line(const char *line)
> +{
> +   if (!strcmp(line, "") || !strcmp(line, "\n"))

>From our in-office discussion:
v1/v0 packs pktlines twice in http, which is not possible to
construct using this test helper when using the same string
for the packed and unpacked representation of flush and delim packets,
i.e. test-pkt-line --pack $(test-pkt-line --pack ) would produce
'' instead of '0009\n'.
To fix it we'd have to replace the unpacked versions of these pkts to
something else such as "FLUSH" "DELIM".

However as we do not anticipate the test helper to be used in further
tests for v0, this ought to be no big issue.
Maybe someone else cares though?

[PATCH v3 11/35] test-pkt-line: introduce a packet-line test helper

[Brandon Williams]

Introduce a packet-line test helper which can either pack or unpack an
input stream into packet-lines and writes out the result to stdout.

Signed-off-by: Brandon Williams 
---
 Makefile |  1 +
 t/helper/test-pkt-line.c | 64 
 2 files changed, 65 insertions(+)
 create mode 100644 t/helper/test-pkt-line.c

diff --git a/Makefile b/Makefile
index b7ccc05fa..3b849c060 100644
--- a/Makefile
+++ b/Makefile
@@ -669,6 +669,7 @@ TEST_PROGRAMS_NEED_X += test-mktemp
 TEST_PROGRAMS_NEED_X += test-online-cpus
 TEST_PROGRAMS_NEED_X += test-parse-options
 TEST_PROGRAMS_NEED_X += test-path-utils
+TEST_PROGRAMS_NEED_X += test-pkt-line
 TEST_PROGRAMS_NEED_X += test-prio-queue
 TEST_PROGRAMS_NEED_X += test-read-cache
 TEST_PROGRAMS_NEED_X += test-write-cache
diff --git a/t/helper/test-pkt-line.c b/t/helper/test-pkt-line.c
new file mode 100644
index 0..0f19e53c7
--- /dev/null
+++ b/t/helper/test-pkt-line.c
@@ -0,0 +1,64 @@
+#include "pkt-line.h"
+
+static void pack_line(const char *line)
+{
+   if (!strcmp(line, "") || !strcmp(line, "\n"))
+   packet_flush(1);
+   else if (!strcmp(line, "0001") || !strcmp(line, "0001\n"))
+   packet_delim(1);
+   else
+   packet_write_fmt(1, "%s", line);
+}
+
+static void pack(int argc, const char **argv)
+{
+   if (argc) { /* read from argv */
+   int i;
+   for (i = 0; i < argc; i++)
+   pack_line(argv[i]);
+   } else { /* read from stdin */
+   char line[LARGE_PACKET_MAX];
+   while (fgets(line, sizeof(line), stdin)) {
+   pack_line(line);
+   }
+   }
+}
+
+static void unpack(void)
+{
+   struct packet_reader reader;
+   packet_reader_init(, 0, NULL, 0,
+  PACKET_READ_GENTLE_ON_EOF |
+  PACKET_READ_CHOMP_NEWLINE);
+
+   while (packet_reader_read() != PACKET_READ_EOF) {
+   switch (reader.status) {
+   case PACKET_READ_EOF:
+   break;
+   case PACKET_READ_NORMAL:
+   printf("%s\n", reader.line);
+   break;
+   case PACKET_READ_FLUSH:
+   printf("\n");
+   break;
+   case PACKET_READ_DELIM:
+   printf("0001\n");
+   break;
+   }
+   }
+}
+
+int cmd_main(int argc, const char **argv)
+{
+   if (argc < 2)
+   die("too few arguments");
+
+   if (!strcmp(argv[1], "pack"))
+   pack(argc - 2, argv + 2);
+   else if (!strcmp(argv[1], "unpack"))
+   unpack();
+   else
+   die("invalid argument '%s'", argv[1]);
+
+   return 0;
+}
-- 
2.16.0.rc1.238.g530d649a79-goog