Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Explore Lint based or reflection based tests for struct equality changes #281

Open
suyashkumar opened this issue Aug 26, 2023 · 0 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@suyashkumar
Copy link
Owner

In #280, we introduced Equals() methods for most Dataset related structs, which greatly reduces time to check for equality of large datasets (useful in tests and elsewhere) vs. relying on reflection. However, we must take care that if these structs are modified, the Equals method is also updated. These structs should not change often, so enforcing it at code review for now and this isn't a super high priority. But one could imagine lint comment based rules to check for this or reflection based tests to autogenerate a small set of equal and unequal values for every struct field (and those would fail if a newly added struct field wasn't reflected in the Equals method).

@suyashkumar suyashkumar added enhancement New feature or request help wanted Extra attention is needed labels Aug 26, 2023
@suyashkumar suyashkumar changed the title Explore Lint based or reflection based tests for struct equality Explore Lint based or reflection based tests for struct equality changes Aug 26, 2023
suyashkumar added a commit that referenced this issue Aug 26, 2023
…t rely on dataset comparisons. (#280)

This change introduces well-defined equality methods for Dataset, Element, and some other data structures. This greatly speeds up tests that rely on checking equality of datasets (that previously needed reflection). For example, it reduces the total test suite from 1m24s to 10s on GitHub actions (mostly due to one test). These methods may also be of use to library users.

However, this does mean that if new fields are added to any of these structs it is important for the Equals method to be updated as well. For now this will be enforced during code review (helped by the fact most of these structs should not fail often), but we should investigate lint rules or some auto-generated reflection based tests that can help catch when this doesn't happen (see #281).

This change also makes a change to rely on pointers for []*frame.Frame in the PixelDataInfo.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

1 participant