Add new option should_infer_types to optionally not infer the type of the secret

[cepages]

Description

This PR allows to set an option to not use inference to find out the secret type. If inference_secret_types = false the type will be always String.

Discussion

@rogerluan ( CarlosNano = cepages ) I've created this initial PR, I'm not a ruby developer but this seem to be a plausible way of doing it.

Resources

Resolves #70

[rogerluan]

Hi @cepages 👋

Thank you so much for this! You definitely went in the right direction — congrats on your first open source PR in Ruby! 😁

I just made some minor nitpick comments about the interface of this new config with the developers, as well as keeping the codebase consistent, I hope you don't mind!

Also, you should check the failing unit and integration tests, as the changes you made are okay in isolation but the tests will need to be updated 🙏 to run the tests, you can run bundle exec rspec

Thanks once again and well done! 🙌

[cepages]

bundle exec rspec nice thanks. That's what I was searching. I will do the changes and run the tests

[rogerluan]

👏 Great work @cepages !! Thank you for this contribution!

(Just pending fixing the unit tests 😊)

[cepages]

@rogerluan I keep changing the test files and they keep failing.

For example: templates_arguments_spec.rb, I add:

      expect(subject.instance_variable_get(:@should_infer_types)).to eq config.should_infer_types

in:

  describe ".new" do

also added should_infer_types: true in arakan-fixture.yml

Could you point me how to fix that one and maybe I can replicate in the rest of them :/

[rogerluan]

@cepages you have to read the reason why the specs are failing, not only which specs are failing. For example:

This test is failing because encode! is being called without the argument should_infer_types. To fix this, search where encode! is being used across the project and make sure all of its callers are passing this new argument you added to this function :)

Then repeat the process: read the error for each failing spec and fix them individually until they all pass 🤗

[cepages]

@rogerluan I've fixed the test locally and it seems the Swift test are failing installing dependencies?
An error occurred while installing strscan (3.1.0), and Bundler cannot continue.

Btw thanks for the tip to fix the tests, I was looking it wrong :)

[rogerluan]

@cepages I confirmed that this bug is present in main (https://github.com/rogerluan/arkana/actions/runs/10235693630/job/28316834697), thus it's unrelated to this PR, and it's likely an issue with GitHub Actions. I'll investigate this later, dw :)

Meanwhile, congrats on fixing the existing tests! 🎉

Would you mind adding new tests, asserting that the new behavior you're adding is behaving as expected? That means it should be inferring the types when passing true, and forcing the usage of String (the default type) when passing false, for both generators we have (Swift, Kotlin) 😊

Thanks again! 🙌

[cepages]

Sure, I was tempted to added yesterday but I didn’t see any other test in the encode class to infer the types. Maybe I should add the test other class? Thanks

…

On Sun, 4 Aug 2024 at 12:08, Roger Oba ***@***.***> wrote: @cepages <https://github.com/cepages> I confirmed that this bug is present in main ( https://github.com/rogerluan/arkana/actions/runs/10235693630/job/28316834697), thus it's unrelated to this PR, and it's likely an issue with GitHub Actions. I'll investigate this later, dw :) Meanwhile, congrats on fixing the existing tests! 🎉 Would you mind adding new tests, asserting that the new behavior you're adding is behaving as expected? That means it should be inferring the types when passing true, and forcing the usage of String (the default type) when passing false, for both generators we have (Swift, Kotlin) 😊 Thanks again! 🙌 — Reply to this email directly, view it on GitHub <#72 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABIEENWKOSXF3GIWQFSLP4DZPYDSXAVCNFSM6AAAAABKIPPFZGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENRXGUYDENBRGI> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[rogerluan]

Hmm good point, I analyzed a bit further and probably there's not enough value in adding a unit test for this, but maybe a full integration test would be better, e.g. to make sure the final output produces code that compile and works as expected. What I'd probably do is duplicate the Swift and Kotlin tests in the Rakefile, using an arkana config file that is identical but with should_inter_types set to false, keeping everything else identical. That should guarantee that everything is working well for both generators 🙇

[cepages]

Done it, I hope I've done it in the way you mean in your previous comment. The swift test seems to persist but kotlin seems to pass ok.

It seems a bit of duplication of the files though.

Which leads me to another idea. It would be very cool if arkana in the command line, allows you to override any option from the config file, so we could do something like:

ARKANA_RUNNING_CI_INTEGRATION_TESTS=true arkana --config-filepath #{config_file} --dotenv-filepath #{dotenv_file} --include-environments dev,staging --should_inter_types false

Which would ignore the option for should_inter_types in the config file, forced to false. It would allows us to test any config option much easily.

Just an idea :)

[rogerluan]

Just a couple more minor changes @cepages! 🤗

Your idea about overriding the config from the CLI is great!! I really think we should have that. If you'd like to take a stab at implementing that (probably wiser in a separate PR), I'd be happy to review it! I think it'd indeed be great to allow further customization and remove duplicated fixtures in this project 😅

Thanks again!

[rogerluan]

I ran the rake test_swift locally and it runs just fine 💪 I think this PR is good to go! 🚀

Congrats @cepages on your first contribution being merged! 🤩

[cepages]

Nice!! 🥳

Thanks a lot for your help @rogerluan, I couldn't have done it without you 😄

I will open a MR for the other idea.

Thanks!

[rogerluan]

Glad I could help!

Awesome! 🤩 let me know if you need anything 🤗