Client Compatibility Tests engine #1013
Conversation
|
|
||
| private void doGetObject() { | ||
| try { | ||
| _createBucket(); |
There was a problem hiding this comment.
You don't need to create state for the tests - compatibility tester does it for you. So you can remove _createBucket() and _putObject() here and in other tests and just do _getObject(). So this test can be reduced to:
try {
ObjectStore os = nc.objectStore(bucket);
ByteArrayOutputStream baos = new ByteArrayOutputStream();
_respondDigest(os.get(object, baos));
}There was a problem hiding this comment.
Same goes for all other tests preparing initial state: doUpdateMetadata(), doPutObject() etc.
There was a problem hiding this comment.
Fixing tests for state setup.
|
|
||
| ObjectStoreConfiguration osc = extractObjectStoreConfiguration(); | ||
| try { | ||
| osm.delete(osc.getBucketName()); |
There was a problem hiding this comment.
You don't need that here - compat tester handles the cleanup for you.
There was a problem hiding this comment.
Except if you kill the cli in the middle of a test, the bucket is sometimes left around. This caused much pain, so once I figured it out, I left it in there.
| public void endOfData() {} | ||
| } | ||
|
|
||
| protected void _respondDigest(ObjectInfo oi) { |
There was a problem hiding this comment.
I don't think this validates what it's supposed to validate. You're taking digest from the object you got and just returning it back, so it will match whatever was created by the compatibility tester. What you should do is calculate the SHA for the object you got and return the SHA, not the digest from ObjectInfo.
There was a problem hiding this comment.
As the client gets an object, it calculates the digest and checks that it matches what comes from the server. If the value does not match, I throw an exception. When it does pass the checks, the oi value is the same as what I calculate.
if (totalBytes != oi.getSize()) { throw OsGetSizeMismatch.instance(); }
if (totalChunks != oi.getChunks()) { throw OsGetChunksMismatch.instance(); }
if (!digester.matches(oi.getDigest())) { throw OsGetDigestMismatch.instance(); }
| try { | ||
| ObjectStore os = nc.objectStore(bucket); | ||
| ObjectMeta meta = ObjectMeta.builder(objectName).description(description).build(); | ||
| InputStream inputStream = Utility.getFileAsInputStream("nats-server.zip"); |
There was a problem hiding this comment.
I don't think you should keep nats-server.zip in the reposiotry - compat tester gives you the URL to the thing you should put in the object store. You can simply download whatever's in the received config and use this instead. That way you are not risking having the test fails if at some point the url in config changes (for whatever reason).
There was a problem hiding this comment.
What's the difference between that and hard-coding bucket names in the default test?
There was a problem hiding this comment.
Also, why is there a url in the get-link test?
[p1t8@674140620] CMD
tests.object-store.get-link.get-link.command
{"bucket":"test","suite":"object-store","test":"get-object","checksum":"","command":"get","url":"https:\/\/github.com\/nats-io\/nats-server\/releases\/download\/v2.9.19\/nats-server-v2.9.19-linux-arm64.zip","object":"link"}
There was a problem hiding this comment.
Changed code to download the file from the url in the json
There was a problem hiding this comment.
I think the url in get link is there by accident, it has no value for the client.
There was a problem hiding this comment.
As I said in the other comment, I think it's safer to download from the provided url when needed.
| } | ||
|
|
||
| if (extraStr == null || extraStr.isEmpty()) { | ||
| // if (error) { |
| } | ||
|
|
||
| extraStr = NEWLINE_INDENT + extraStr.replaceAll("\n", NEWLINE_INDENT); | ||
| // if (error) { |
There was a problem hiding this comment.
Looks good, just one suggestion I forgot about earlier.
It would be nice to be able to customize the nats URL when running the test - it is possible in compat tester using --nats-url flag, so would be good to enable running compatibility tests on custom URL. Go and Rust tests utilize NATS_URL env variable for this, which may easily be passed to docker container, which is where those tests are executed.
I added command line and env support for server |
Built client compatibility test framework and implemented object store specific tests. Used in conjunction with an internal program which made to validate consistent client behavior.