vladislav-sidorovich commented on PR #15407: URL: https://github.com/apache/iceberg/pull/15407#issuecomment-6543343515
@kevinjqliu thank you for the feedback. 1. I removed jackson. 2. I added silently skip to keep the build more robust, but now I think you are right, it should be explicit fail instead. Fixed. **Unit tests** > Can we generate the tables at runtime instead? I believe it's wrong approach in unit-tests because it introduced generator which can be buggy. Delta protocol specify tables structure, not clients (write path) behavior. So we need `static` files to verify all scenarios according to the protocol. We need something like this iceberg repo: https://github.com/apache/iceberg-verification. I see 2 options: 1. Have table files in the repo - it was declined 2. Download the table files as an artifact 2.1 DAT repository was the first solution I found 2.2 I can build a jar deps and upload it to maven repo. **Integration tests** In the integration tests, we have the generation. The difference is that integration test cover bigger E2E scenario and do not cover all the edge cases supported by speck. (for example inline DVs or bin files). **Development process** It can be not visible from the tests, but these unit-tests were very useful during development 90%+ time I used unit-tests to verify the code correctness. For example `CommitInfo` is an optional Delta action and we can't rely on it (as was noticed in the comments), but Spark aways write this action. **Next steps** I see why you thin that dependency on another github repo can be risky. What about dependency on new a new artifact from Maven repo? I can publish the tables to maven, and download them during tests as a regular dependency (json, parquet). @kevinjqliu please let me know what do you think, so I will proceed. P.S. Sorry for the delay in responding; I can dedicate the weekends to this project. -- 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]
