nastra commented on PR #14351: URL: https://github.com/apache/iceberg/pull/14351#issuecomment-3436153632
@gaborkaszab It takes me **1min 17 secs** to run `TestRewriteTablePathsAction` on a MBP, so **4min 55secs** seems super long to me for one dimension of testing. > In other words there is no test exercising the new code changes even with adding the new test dimension for format-version This is because calls to `FileHelpers.writePosDeleteFile` / `FileHelpers.writeDeleteFile` have not been adjusted to pass the `formatVersion` in. @adawrapub can you please fix that? @adawrapub also just preserving the DVs won't make the test suite pass automatically, because there's another [issue](https://github.com/apache/iceberg/issues/13671#issuecomment-3130889106) that needs to be fixed so that rewriting table paths fully works with v3. > My proposal is to identify the subset of tests where the format version is relevant now, practically the ones having delete files, and add test param for them, but not for the entire suite. Unless I miss something The scope of https://github.com/apache/iceberg/issues/13671 is to fix DVs with rewriting table paths and it might seem ok to only parameterize the test methods that write DVs but it's possible that we're missing other subtle bugs around rewriting (since we're always doing this with v2 only), hence I'm advocating to parameterize the format version at the class level to make sure we're not missing anything. However, I understand the concern around long running test suites with a large test matrix, so we still need to keep this in mind and maybe selectively skip tests for certain format versions. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
