Skip to content

Conversation

grzuy
Copy link
Contributor

@grzuy grzuy commented Sep 22, 2025

Contributor checklist

Leave anything that you believe does not apply unchecked.

  • AI was not used in the creation of this PR.
  • Bug fixes include regression tests
  • Chores
  • Documentation changes
  • Features include unit/acceptance tests
  • Refactoring
  • Update dependencies

Hey @zachdaniel 👋

I spotted this issue while working on mimiquate/tower#168.

And I was able to reproduce it here with this test case. But to be honest I am not sure I am calling the right functions and with the correct expectations.

I debugged a bit and the failure lead me to a few changes around Aug 2024...

Thanks in advance!

@grzuy grzuy changed the title test: modify_config_code twice with keyword values test: modify_config_code twice with keyword values Sep 22, 2025
@grzuy grzuy changed the title test: modify_config_code twice with keyword values test: modify_config_code twice with keyword values (failing) Sep 22, 2025
@grzuy
Copy link
Contributor Author

grzuy commented Sep 22, 2025

It seems that setting format: :keyword to the ast node in the final case clause when setting the keyword value would fix the test

diff --git a/lib/igniter/code/keyword.ex b/lib/igniter/code/keyword.ex
index 57e7a2f..7448017 100644
--- a/lib/igniter/code/keyword.ex
+++ b/lib/igniter/code/keyword.ex
@@ -167,11 +167,8 @@ defmodule Igniter.Code.Keyword do
                   {{:__block__, [], [key]}, {:__block__, [], [nil]}}
                 end
 
-              [] ->
-                {{:__block__, [format: :keyword], [key]}, {:__block__, [], [nil]}}
-
               _current_node ->
-                {{:__block__, [], [key]}, {:__block__, [], [nil]}}
+                {{:__block__, [format: :keyword], [key]}, {:__block__, [], [nil]}}
             end
 
           {:ok, zipper} =

but honestly not sure this is the correct way to go nor if it has any unintended consequences.

@zachdaniel
Copy link
Contributor

If making that change fixes this test and doesn't break anything else, then I'm all for it ❤️

@zachdaniel
Copy link
Contributor

Would you want to include that fix here?

@grzuy grzuy changed the title test: modify_config_code twice with keyword values (failing) fix: modify_config_code twice with keyword values Sep 25, 2025
@grzuy
Copy link
Contributor Author

grzuy commented Sep 25, 2025

Thanks for the feedback.

Included extra commit with that change.

Running mix test locally I only get 1 failure which I also get when running on main, so it might be something else about my local setup.

Workflow here awaiting approval to run. We'll see 🙂

@zachdaniel zachdaniel merged commit 4dbeab1 into ash-project:main Sep 25, 2025
39 checks passed
@zachdaniel
Copy link
Contributor

🚀 Thank you for your contribution! 🚀

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