pvary commented on PR #10449:
URL: https://github.com/apache/iceberg/pull/10449#issuecomment-2151612132
> I am not sure how to add the test & also think no need add the test.
I disagree with you here because of 2 reasons:
1. If there is no tests, then how can we be sure that future changes around
the code doesn't revert back your fix?
2. If we don't have tests executing the code path, we can't be sure that the
code is working as expected
Minimally we need to be sure that both codepath is called at least once
during the tests. Could you please provide an example which tests executes the
following line:
```
scan = scan.project(SchemaParser.fromJson(schemaStr));
```
also, which test executes this line:
```
scan =
scan.select(conf.getStrings(InputFormatConfig.SELECTED_COLUMNS));
```
You might be able to create our own test to verify that the functions are
called by adding a new test which tries to set both `project` and `select`, and
fails because of the line you have linked.
If we verified that the lines are called in our existing tests, and we have
new test which verifies that the values are set on the scan then I'm fairly
confident that we will not have regression later around the codepaths you're
fixing now.
Thanks for your work!
Peter
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
