Skip to content

build: Use common libraries#32

Merged
Annopaolo merged 2 commits into
secomind:mainfrom
noaccOS:push-zxsqrmwukvvm
Feb 26, 2026
Merged

build: Use common libraries#32
Annopaolo merged 2 commits into
secomind:mainfrom
noaccOS:push-zxsqrmwukvvm

Conversation

@noaccOS
Copy link
Copy Markdown
Contributor

@noaccOS noaccOS commented Feb 25, 2026

  • use typedstruct instead of typed_struct to avoid mismatches with other astarte libraries
  • remove efx as it depends on typed_struct, and use mimic in its stead

Copy link
Copy Markdown
Collaborator

@Annopaolo Annopaolo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code is good, I left some very nitpicking comments

}
end

defp start_amqp_data_consumer_fixture!(queue_index) do
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this name might be clearer. If it means "start the amqp_data_consumer fixture", then "set up" is the better verb; if it means "a fixture that starts the amqp_data_consumer", then the fixture part is redundant and "start_amqp_data_consumer" is better.
I would suggest something like start_amqp_data_consumer!() in the same fashion as the usual start_link.

This is very minor though

Comment thread test/consumer/data_updater_test.exs Outdated
Comment on lines +172 to +175
{:ok, data_updater} = DataUpdater.get_data_updater_process(sharding_key)
:erlang.trace(data_updater, true, [:receive])
Mimic.allow(MessageTracker, self(), data_updater)
allow(MockMessageHandler, self(), data_updater)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this setup_data_updater/1 from lines 223-231?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, after having set the expectations
I've moved expectations to a setup so that we can reuse the existing setup

Comment on lines +333 to +347
defp setup_data_updater(context) do
%{sharding_key: sharding_key} = context

{:ok, data_updater} =
GenServer.start_link(DataUpdater,
sharding_key: sharding_key,
message_handler: MockMessageHandler
)

:erlang.trace(data_updater, true, [:receive])

Mimic.allow(MessageTracker, self(), data_updater)
allow(MockMessageHandler, self(), data_updater)

%{data_updater: data_updater}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is repeated in similar fashion in two or three files, would it be reasonable to write it only once somewhere in the test dir and simply reference it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, though they're still two separate functions

- use typedstruct instead of typed_struct to avoid mismatches with
  other astarte libraries
- remove efx as it depends on typed_struct, and use mimic in its stead

Signed-off-by: Francesco Noacco <francesco.noacco@secomind.com>
Signed-off-by: Francesco Noacco <francesco.noacco@secomind.com>
@Annopaolo Annopaolo merged commit a76f9f0 into secomind:main Feb 26, 2026
3 checks passed
@noaccOS noaccOS deleted the push-zxsqrmwukvvm branch February 26, 2026 14:31
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