You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "Elbehery (via GitHub)" <gi...@apache.org> on 2025/07/26 14:48:44 UTC
[PR] feat(test): add edge cases tests [iceberg-go]
Elbehery opened a new pull request, #502:
URL: https://github.com/apache/iceberg-go/pull/502
This PR adds tests for error and edge cases.
cc @laskoviymishka @zeroshade
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] test: add edge cases tests [iceberg-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on PR #502:
URL: https://github.com/apache/iceberg-go/pull/502#issuecomment-3123556562
> existing tests in table_test.go or other tests ?
Either. There isn't much reason to add these tests unless they are cases which aren't already being tested.
> also other PRs I have raised contains test that could be duplicated :)
Can you please remove duplicate tests from those PRs also?
You've raised 15 PRs in 12 hours. That isn't sustainable and I'm guessing that an AI tool was involved here, right? Can you please pare down the PRs so that they properly follow the style in the repo and aren't duplicating tests and testing suites that already exist?
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] test: add edge cases tests [iceberg-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on PR #502:
URL: https://github.com/apache/iceberg-go/pull/502#issuecomment-3123531052
What conditions are being tested that aren't already covered by the existing tests? Why create a new test suite that duplicates many of the helper functions instead of just adding to the existing one?
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] test: add edge cases tests [iceberg-go]
Posted by "Elbehery (via GitHub)" <gi...@apache.org>.
Elbehery commented on PR #502:
URL: https://github.com/apache/iceberg-go/pull/502#issuecomment-3123578543
> > existing tests in table_test.go or other tests ?
>
> Either. There isn't much reason to add these tests unless they are cases which aren't already being tested.
>
> > also other PRs I have raised contains test that could be duplicated :)
>
> Can you please remove duplicate tests from those PRs also?
>
> You've raised 15 PRs in 12 hours. That isn't sustainable and I'm guessing that an AI tool was involved here, right? Can you please pare down the PRs so that they properly follow the style in the repo and aren't duplicating tests and testing suites that already exist?
> Can you please remove duplicate tests from those PRs also?
sure 👍🏽
> AI tool was involved here
is this an issue ? I use it to understand the java repo and find the missing tests between here and there
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] test: add edge cases tests [iceberg-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on PR #502:
URL: https://github.com/apache/iceberg-go/pull/502#issuecomment-3123592976
> is this an issue ? I use it to understand the java repo and find the missing tests between here and there
That explains the duplicate tests and infrastructure, along with the problems in the code. Please don't raise PRs that are entirely generated by an AI tool without having an understanding of the code and tests yourself. We don't have the bandwidth to sift through AI generated code in the hopes for something salvageable.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] test: add edge cases tests [iceberg-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on PR #502:
URL: https://github.com/apache/iceberg-go/pull/502#issuecomment-3123542864
That doesn't answer my question, can you point me to anything here that isn't already covered by existing tests?
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] test: add edge cases tests [iceberg-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade closed pull request #502: test: add edge cases tests
URL: https://github.com/apache/iceberg-go/pull/502
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] test: add edge cases tests [iceberg-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on PR #502:
URL: https://github.com/apache/iceberg-go/pull/502#issuecomment-3123607399
I'm going to close this PR for now. If you want to update and fix this PR then you can reopen it and I'll review.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] test: add edge cases tests [iceberg-go]
Posted by "Elbehery (via GitHub)" <gi...@apache.org>.
Elbehery commented on PR #502:
URL: https://github.com/apache/iceberg-go/pull/502#issuecomment-3123533888
> What conditions are being tested that aren't already covered by the existing tests? Why create a new test suite that duplicates many of the helper functions instead of just adding to the existing one?
thanks for your review
i was trying to test edge cases, inspired by Java repo
we can merge what you find convenient and skip the rest 👍🏽
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] test: add edge cases tests [iceberg-go]
Posted by "Elbehery (via GitHub)" <gi...@apache.org>.
Elbehery commented on PR #502:
URL: https://github.com/apache/iceberg-go/pull/502#issuecomment-3123551795
> That doesn't answer my question, can you point me to anything here that isn't already covered by existing tests?
existing tests in table_test.go or other tests ?
also other PRs I have raised contains test that could be duplicated :)
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] test: add edge cases tests [iceberg-go]
Posted by "Elbehery (via GitHub)" <gi...@apache.org>.
Elbehery commented on PR #502:
URL: https://github.com/apache/iceberg-go/pull/502#issuecomment-3123598859
> > is this an issue ? I use it to understand the java repo and find the missing tests between here and there
>
> That explains the duplicate tests and infrastructure, along with the problems in the code. Please don't raise PRs that are entirely generated by an AI tool without having an understanding of the code and tests yourself. We don't have the bandwidth to sift through AI generated code in the hopes for something salvageable.
thanks for your review :)
just i will raise the last PR, as it is already done :)
thanks for your patience :)
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org