Uh oh!
There was an error while loading. Please reload this page.
Parquet: Remove duplicate test code - #8098
Conversation
nastra
commented
Jul 19, 2023
@ajantha-bhat can you rebase please? |
ajantha-bhat
commented
Jul 19, 2023
Done. I would have saved some effort on #8056 if I have found it earlier. |
coded9
commented
Jul 20, 2023
@ajantha-bhat I found it the day before merging the PR when I observed package collisions. I wanted to handle it in a separate PR after discussion with @nastra. Thanks for this PR. |
| protected GenericData.Record writeAndRead( | ||
| String desc, Schema writeSchema, Schema readSchema, GenericData.Record record) | ||
| throws IOException { | ||
| File file = temp.resolve(desc + ".parquet").toFile(); |
There was a problem hiding this comment.
This was modified by #8056.
Hence, reverting this line and making it compatible with parent class in iceberg-core
There was a problem hiding this comment.
It would be better to first switch TestReadProjection from iceberg-core to JUnit5 and then removing TestReadProjection from iceberg-parquet
There was a problem hiding this comment.
That may require moving to Junit5 for a whole iceberg-core module. I am afraid I don't have a time to work on that.
@coded9, if you are intersted, please feel free to work on it.
I was doing a POC of partition stats where I had to move all the files of iceberg-parquet into iceberg-core, that time I found this duplicate code.
There was a problem hiding this comment.
Done. Rebased. Now it is on Junit 5.
| // Empty struct read is not supported for Parquet | ||
| @Override | ||
| public void testEmptyStructProjection() throws Exception {} |
There was a problem hiding this comment.
These tests was added for Avro. When empty struct was supported. But it is not yet supported for parquet. Since both avro and parquet now use the common class. Ignoring the tests for parquet.
There was a problem hiding this comment.
Also, This class uses (extends) TestReadProjection from api module now. Instead of duplicated local copy.
Uh oh!
There was an error while loading. Please reload this page.
TestReadProjection.javainiceberg-parquetmodule is an exact duplicate oficeberg-coremodule'sTestReadProjection.java.iceberg-parquetmodule can directly extend a test class fromiceberg-coreas it already depends on test artifacts.Note: Empty struct is supported for
iceberg-coreavro readers (#2953).That time 5 new testcases were added. which won't work for parquet readers.