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

Copy structs when they exist #67

Merged
merged 1 commit into from
Jun 21, 2024
Merged

Conversation

harrisi
Copy link
Contributor

@harrisi harrisi commented Jun 19, 2024

Fixes #65 (and I think #55, and partially #28 maybe?).

I'm not sure if this is the proper fix, or if the test is ideal. Happy to modify things if needed.

@edgurgel edgurgel merged commit f073afb into edgurgel:main Jun 21, 2024
3 checks passed
@edgurgel
Copy link
Owner

Thank you! I will do some testing and release a new version cheers!

@harrisi
Copy link
Contributor Author

harrisi commented Jun 21, 2024

I figure I should mention, I'm not copying which fields are required or not, so there's no way to test construction of a struct is done with required fields. That didn't seem like something anyone has opened an issue about, so I left it out of this PR.

It would maybe make sense to do that, but I don't actually use Mimic so I'm not sure if that's outside the scope of this package.

@harrisi harrisi deleted the copy-struct branch June 21, 2024 23:07
@edgurgel
Copy link
Owner

It would maybe make sense to do that, but I don't actually use Mimic so I'm not sure if that's outside the scope of this package.

Yeah I'm not too sure if the required keys is needed at the time that Mimic runs. It might be worth doing just in case someone is relying on getting the metadata about structs and expect to see the same info with and without Mimic I guess 🤷

@edgurgel
Copy link
Owner

In any case this should do the job: 816fd77

@jimsynz
Copy link
Contributor

jimsynz commented Jul 18, 2024

This caused some intermittent test failures for me where dynamically creating a struct with Kernel.struct/2 from a copied module. In my case the copied module only had a single assertion against it so I was able to remove it. I couldn't figure out how to reproduce it, otherwise I'd open a PR so I'm leaving a comment here in case other folks hit it.

Related commit is ash-project/reactor@e4d73d3

@edgurgel
Copy link
Owner

Thanks, @jimsynz that's very useful! 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Intermittent runtime error thrown in Elixir 1.17 / OTP 27
3 participants