[BUG] Requires live mode messes with data in certain scenarios.

[rickle-msft]

Given the following test helper method:

def compareDataToFile(Flux<ByteBuffer> data, File file) {
        FileInputStream fis = new FileInputStream(file)

        for (ByteBuffer received : data.toIterable()) {
            byte[] readBuffer = new byte[received.remaining()]
            fis.read(readBuffer)
            for (int i = 0; i < received.remaining(); i++) {
                if (readBuffer[i] != received.get(i)) {
                    return false
                }
            }
        }

        fis.close()
        return true
    }

being called from a test that downloads a blob and passes the returned Flux in along with the sourceFile that was uploaded produces different results depending on the presence of the @requires( {liveMode()} ) annotation.

For dataSizes over a few MB, the test will consistently fail. More specifically, what appears to happen is that the presence of the annotation causes the items coming from the result of toIterable to be out of order. Small data sizes do not fail because there is only one emission, but anything with more than one emission will fail. The data downloaded is correct (which can be observed by downloading to a file and comparing those results instead). The test behavior changes reliably by toggling the presence of this annotation, but it is not clear why.

[sima-zhu]

Taking a look

[sima-zhu]

The issue can be reproduced.

The test has been experiment is:
def "Encrypted upload file"() of test class EncyptedBlockBlobAPITest under azure-storage-blob-cryptography module.

Test scenarios:

• Size: KB. The test passed.
• Size: MB. With @requires( {liveMode()} ) annotation. The test failed.
• Size: MB. Remove @requires( {liveMode()} ) annotation. The test passed.

[sima-zhu]

@requires({true}) is not working with the DirectByteBuffer correctly.
However, tried on @IgnoreIf({!liveMode}) is working well with the download API tests.

If it is necessary to have tests on more than 1MB data, suggested to switch to @IgnoreIf annotation in Spock

[sima-zhu]

As this is a conflict between spock annotation and nio bytebuffer, there is limited room for SDK to change anything.

[rickle-msft]

Thanks for investigating this! I don't think we'll have any problem with just using the right annotations, even if it's a little weird... I can't remember right now, but do you know if there are tests that currently hit this problem using @requires({ liveMode() })?

[sima-zhu]

There is an open PR which added DoNotRecord annotation. Will test the problem again if the annotation is online.

#6134

[sima-zhu]

@rickle-msft

I do not know any existing tests have this kind of issue. The tests under live mode seems working fine in my local.

[alzimmermsft]

Instead of changing the annotations could there be a better way to write the verification code? Instead could we use Reactor's StepVerifier which is used for testing validation?

def compareDataToFile(Flux<ByteBuffer> data, File file) {
        // Use AsynchronousFileChannel as it is part of NIO
        def fileChannel = AsynchronousFileChannel.open(file.toPath(), StandardOpenOption.READ)
        def readOffset = [ 0 ]

        StepVerifier.create(data)
            .thenConsumeWhile({
                def readBuffer = ByteBuffer.allocate(it.remaining())
                // Cheap validation that we read as much as we expected
                assert it.remaining() == fileChannel.read(readBuffer, readOffset[0]).get()
                // Compare buffers instead of individual bytes
                assert it == readBuffer

                readOffset[0] += it.remaining()
                return true
            })
            .verifyComplete()

        fileChannel.close()
        return true
    }

[rickle-msft]

It's worth a shot and may be worth doing anyway

[sima-zhu]

I have tried to use the StepVerifier, and it is not working. But it is actually better to rewrite.

@alzimmermsft Could you try use your new annotation in your PR and test the data with 1MB to see if your new annotation is working under the case

[alzimmermsft]

I've done more investigation and haven't been able to get a consistent replication on this issue. It does fail occasionally but not always.

[sima-zhu]

Based on my testing, it failed most of the time when use @requires.