-
Notifications
You must be signed in to change notification settings - Fork 56
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
Split protocol testing into separate ITs for zilla dump command #989
Conversation
ab250ab
to
79d1ecb
Compare
...and-dump/src/test/java/io/aklivity/zilla/runtime/command/dump/internal/airline/DumpRule.java
Outdated
Show resolved
Hide resolved
...and-dump/src/test/java/io/aklivity/zilla/runtime/command/dump/internal/airline/DumpRule.java
Outdated
Show resolved
Hide resolved
...and-dump/src/test/java/io/aklivity/zilla/runtime/command/dump/internal/airline/DumpRule.java
Outdated
Show resolved
Hide resolved
...test/java/io/aklivity/zilla/runtime/command/dump/internal/airline/GrpcServerStreamRpcIT.java
Outdated
Show resolved
Hide resolved
.../test/java/io/aklivity/zilla/runtime/command/dump/internal/airline/Http1AuthorizationIT.java
Outdated
Show resolved
Hide resolved
...dump/src/test/java/io/aklivity/zilla/runtime/command/dump/internal/airline/TsharkRunner.java
Outdated
Show resolved
Hide resolved
...and-dump/src/test/java/io/aklivity/zilla/runtime/command/dump/internal/airline/DumpRule.java
Outdated
Show resolved
Hide resolved
...src/test/java/io/aklivity/zilla/runtime/command/dump/internal/airline/DumpCommandRunner.java
Outdated
Show resolved
Hide resolved
...src/test/java/io/aklivity/zilla/runtime/command/dump/internal/airline/DumpCommandRunner.java
Outdated
Show resolved
Hide resolved
.../test/java/io/aklivity/zilla/runtime/engine/test/internal/k3po/ext/behavior/ZillaEngine.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like we are getting closer, just the remaining extension types to verify correct via tshark and zilla.lua dissector.
<exclude>**/keys</exclude> | ||
<exclude>**/trust</exclude> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<exclude>**/keys</exclude> | |
<exclude>**/trust</exclude> | |
<exclude>src/test/democa/**/*</exclude> |
int offset, | ||
int length) | ||
{ | ||
boolean shouldWrite = channel.getConfig().hasWriteClosed() || !channel.engine.serverClosed().get(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The second part of this condition is effectively using a global variable on the engine as a whole which doesn't seem quite right.
What problem were we trying to solve with that? What breaks when it is removed?
void setWriteClosed(boolean timestamps); | ||
|
||
boolean hasWriteClosed(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
void setWriteClosed(boolean timestamps); | |
boolean hasWriteClosed(); | |
void setCloseable(boolean closeable); | |
boolean isCloseable(); |
@@ -46,6 +46,7 @@ public final class ZillaTypeSystem implements TypeSystemSpi | |||
public static final TypeInfo<Long> OPTION_AFFINITY = new TypeInfo<>("affinity", Long.class); | |||
public static final TypeInfo<Byte> OPTION_CAPABILITIES = new TypeInfo<>("capabilities", Byte.class); | |||
public static final TypeInfo<String> OPTION_TIMESTAMPS = new TypeInfo<>("timestamps", String.class); | |||
public static final TypeInfo<String> OPTION_WRITE_CLOSED = new TypeInfo<>("writeClosed", String.class); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public static final TypeInfo<String> OPTION_WRITE_CLOSED = new TypeInfo<>("writeClosed", String.class); | |
public static final TypeInfo<String> OPTION_CLOSEABLE = new TypeInfo<>("closeable", String.class); |
@@ -95,6 +96,7 @@ public ZillaTypeSystem() | |||
acceptOptions.add(OPTION_ALIGNMENT); | |||
acceptOptions.add(OPTION_CAPABILITIES); | |||
acceptOptions.add(OPTION_TIMESTAMPS); | |||
acceptOptions.add(OPTION_WRITE_CLOSED); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
acceptOptions.add(OPTION_WRITE_CLOSED); | |
acceptOptions.add(OPTION_CLOSEABLE); |
@@ -114,6 +116,7 @@ public ZillaTypeSystem() | |||
connectOptions.add(OPTION_AFFINITY); | |||
connectOptions.add(OPTION_CAPABILITIES); | |||
connectOptions.add(OPTION_TIMESTAMPS); | |||
connectOptions.add(OPTION_WRITE_CLOSED); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
connectOptions.add(OPTION_WRITE_CLOSED); | |
connectOptions.add(OPTION_CLOSEABLE); |
@Override | ||
public void setWriteClosed( | ||
boolean writeClosed) | ||
{ | ||
this.writeClosed = writeClosed; | ||
} | ||
|
||
@Override | ||
public boolean hasWriteClosed() | ||
{ | ||
return writeClosed; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Override | |
public void setWriteClosed( | |
boolean writeClosed) | |
{ | |
this.writeClosed = writeClosed; | |
} | |
@Override | |
public boolean hasWriteClosed() | |
{ | |
return writeClosed; | |
} | |
@Override | |
public void setCloseable( | |
boolean closeable) | |
{ | |
this.closeable = closeable; | |
} | |
@Override | |
public boolean isCloseable() | |
{ | |
return closeable; | |
} | |
else if (OPTION_WRITE_CLOSED.getName().equals(key)) | ||
{ | ||
setWriteClosed(Boolean.parseBoolean(Objects.toString(value, "false"))); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
else if (OPTION_WRITE_CLOSED.getName().equals(key)) | |
{ | |
setWriteClosed(Boolean.parseBoolean(Objects.toString(value, "false"))); | |
} | |
else if (OPTION_CLOSEABLE.getName().equals(key)) | |
{ | |
setCloseable(Boolean.parseBoolean(Objects.toString(value, "false"))); | |
} |
|
||
public DefaultZillaChannelConfig() | ||
{ | ||
super(); | ||
setBufferFactory(NATIVE_BUFFER_FACTORY); | ||
setTimestamps(true); | ||
setWriteClosed(true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
setWriteClosed(true); | |
setClosable(true); |
@@ -51,12 +52,14 @@ public class DefaultZillaChannelConfig extends DefaultChannelConfig implements Z | |||
private long affinity; | |||
private byte capabilities; | |||
private boolean timestamps; | |||
private boolean writeClosed; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private boolean writeClosed; | |
private boolean closeable; |
Description
This change splits the zilla dump command testing into separate ITs per protocol, with k3po scripts used to drive the engine and produce the engine frames used to generate the packet capture, then a text-based comparison is used to make diffs more readable should an IT fail.
Fixes #958