search

apache / iceberg

Apache Iceberg
https://iceberg.apache.org/
Apache License 2.0
5.85k stars 2.06k forks source link

`TestS3RestSigner` is flaky #10599

Open snazy opened 2 days ago

snazy commented 2 days ago

Apache Iceberg version

main (development)

Query engine

None

Please describe the bug 🐞

The 

TestS3RestSigner > executionError FAILED
    org.opentest4j.AssertionFailedError: [should not have any expired token that required a refresh] 
    expected: 0L
     but was: 1L
        at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
        at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
        at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
        at org.apache.iceberg.aws.s3.signer.TestS3RestSigner.afterClass(TestS3RestSigner.java:120)

/cc @nastra

amogh-jahagirdar commented 1 day ago

I took a brief look, seems like the assertions depend on all of the tests completing within 100 seconds. https://github.com/apache/iceberg/blob/main/aws/src/test/java/org/apache/iceberg/aws/s3/signer/TestS3RestSigner.java#L110. I verified this is the case by just adding a sleep in a test.

It seems like the purpose of the tests is to validate behavior around token refresh. Ideally we shouldn't rely on the timing of the test completion for this, perhaps there is a more deterministic way of testing what we want to test. cc @nastra