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

Check REGENERATE_TYPED_TESTS environment variable to make working with test_generate_typed easier #167

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

davebenvenuti
Copy link
Contributor

A small QOL enhancement to ProtoBoeuf::CodeGenTest#test_generate_types that will regenerate test/fixtures/typed_test.correct.rb if ENV["REGENERATE_TYPED_TESTS"] == "true".

@rwstauner
Copy link
Contributor

Hmm... I like automating things, but I feel like this enables a situation that makes the test worthless... you may as well just skip the test or remove it if it's going to generate both sides of the comparison.
We're also losing the "someone verify this" step but I guess we'd still have eyes on it in the pull request review.

What's the intention here?

  • If you're trying to ignore the test in development, should we just skip if ENV["SKIP_TYPED_TEST"] ?
  • If you're using this as a mechanism for regenerating that file, should we just add it to the Rakefile?

@davebenvenuti
Copy link
Contributor Author

If you're using this as a mechanism for regenerating that file, should we just add it to the Rakefile?

This was the motivation here. I was inspired by a pattern I've seen with the VCR gem where you define an environment variable to rerecord cassettes.

I get your concerns. It would be better if we could somehow enforce that the developer has manually inspected the new fixture. I like your suggestion about a rake task, that feels more appropriate. 👍

@davebenvenuti davebenvenuti marked this pull request as draft January 8, 2025 21:59
@davebenvenuti
Copy link
Contributor Author

I spent maybe 15 minutes trying to improve this test and actually assert that our type definitions look as we expect and discovered our generated code with Sorbet signatures doesn't actually eval: #169

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.

2 participants