Fix FileInStreamIntegrationTest #18178
Conversation
|
Is this test broken at this point? |
yes |
dora/tests/integration/src/test/java/alluxio/client/fs/io/FileInStreamIntegrationTest.java
Show resolved
Hide resolved
dora/tests/integration/src/test/java/alluxio/client/fs/io/FileInStreamIntegrationTest.java
Show resolved
Hide resolved
| @Test(timeout = 10000) | ||
| @LocalAlluxioClusterResource.Config( | ||
| confParams = {PropertyKey.Name.WORKER_BLOCK_HEARTBEAT_INTERVAL_MS, "2000"}) | ||
| public void asyncCacheFirstBlock() throws Exception { |
There was a problem hiding this comment.
async cache feature has been deprecated and removed
| @Test(timeout = 10000) | ||
| @LocalAlluxioClusterResource.Config( | ||
| confParams = {PropertyKey.Name.WORKER_BLOCK_HEARTBEAT_INTERVAL_MS, "2000"}) | ||
| public void asyncCacheAfterSeek() throws Exception { |
There was a problem hiding this comment.
async cache feature has been deprecated and removed
| } | ||
|
|
||
| @Test(timeout = 10000) | ||
| public void asyncCacheFirstBlockPRead() throws Exception { |
There was a problem hiding this comment.
async cache feature has been deprecated and removed
There was a problem hiding this comment.
let's leave this comment open here so if anyone traces the code to this PR they will know what's going on
| } | ||
|
|
||
| @Test | ||
| public void syncCacheFirstBlock() throws Exception { |
There was a problem hiding this comment.
@dbw9580 do you know if this test should still be relevant? I figure it's the getFileBlockInfos() is somehow a deprecated API? But we should still fix this one?
There was a problem hiding this comment.
it basically requires the first portion of the file is cached when the length of the read is larger than the size of a block. The requirement still applies to page store, but it may not be able to report block level information. I suggest to use the cache usage API on worker's cache manager directly to verify the size of the file being cached. something like
CacheManager cm = worker.getCacheManager(); // <- this method may not exist yet
long usedBytes = cm.getUsage()
.partitionedBy(file(uri))
.map(CacheUsage::used)
.orElse(0);
assertEquals(data.length, usedBytes)There was a problem hiding this comment.
let's remove this test for now as it's still using Block API. We can add a new one.
| @Test | ||
| @LocalAlluxioClusterResource.Config( | ||
| confParams = {PropertyKey.Name.USER_STREAMING_READER_CHUNK_SIZE_BYTES, "64KB"}) | ||
| public void readTest1() throws Exception { |
There was a problem hiding this comment.
why is this test irrelevant?
| Assert.assertTrue(value >= 0); | ||
| Assert.assertTrue(value < 256); | ||
| Assert.assertTrue(value >= 0 && value < 256); | ||
| // Assert.assertTrue(value < 256); |
| @@ -144,7 +135,7 @@ private List<CreateFilePOptions> getOptionSet() { | |||
| @LocalAlluxioClusterResource.Config( | |||
| confParams = {PropertyKey.Name.USER_STREAMING_READER_CHUNK_SIZE_BYTES, "64KB"}) | |||
| public void readTest1() throws Exception { | |||
| for (int k = MIN_LEN; k <= MAX_LEN; k += DELTA) { | |||
| for (int k = MIN_LEN; k <= MAX_LEN; k += 3 * BLOCK_SIZE) { | |||
There was a problem hiding this comment.
| for (int k = MIN_LEN; k <= MAX_LEN; k += 3 * BLOCK_SIZE) { | |
| // use a larger step so the test runs faster | |
| for (int k = MIN_LEN; k <= MAX_LEN; k += 3 * BLOCK_SIZE) { |
| } | ||
|
|
||
| @Test | ||
| public void syncCacheFirstBlock() throws Exception { |
There was a problem hiding this comment.
let's remove this test for now as it's still using Block API. We can add a new one.
|
alluxio-bot, merge this please |
Fix `FileInStreamIntegrationTest`, removed unnecessary test cases pr-link: Alluxio#18178 change-id: cid-dac1c4c2e4e403bbf4ad3cbdde9eaf43ce20e046
Fix
FileInStreamIntegrationTest, removed unnecessary test cases