Skip to content

Conversation

Zerpet
Copy link
Member

@Zerpet Zerpet commented Sep 29, 2025

This pull request refactors the handling of queue and exchange types in the RabbitMQ AMQP package, simplifying type usage and improving test reliability. The main changes include removing wrapper structs for queue and exchange types in favor of using plain type aliases, updating all related interfaces and methods, and refactoring tests to use better setup/teardown patterns and more idiomatic assertions.

Refactoring of queue and exchange type handling

  • Removed the QueueType and ExchangeType wrapper structs and their associated .String() methods, replacing them with direct usage of the TQueueType and TExchangeType type aliases throughout the codebase. All interfaces, specifications, and related methods now use these aliases directly. [1] [2]
  • Updated all queue and exchange specification structs and their methods to use the new type aliases, simplifying the code and removing unnecessary conversions. [1] [2] [3] [4] [5] [6] [7] [8]
  • Changed how queue and exchange types are set in arguments maps, using direct string conversion from the type alias instead of calling .String(). [1] [2] [3] [4] [5]

Updates to queue and exchange classes

  • Refactored the AmqpExchange and AmqpQueue classes to use the new type aliases for setting and getting types, simplifying their APIs and internal logic. [1] [2] [3]

Test improvements

  • Refactored tests to remove unnecessary use of sync.WaitGroup, replacing it with BeforeEach and AfterEach hooks for setup and cleanup, and improved assertion style to use Expect(...).ToNot(HaveOccurred()) for error checks. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10]
  • Improved import organization and removed unused imports in test files. [1] [2]

It is just a wrapper around TQueueType
Assertions should be made only inside subject nodes i.e. It(), otherwise, a failed assertion panics and the test code does not recover correctly.
It had a wrapper struct. It's simpler to use the exchange type directly
Because it can cause panics on failed assertions, and it does not respect the focus marker i.e. it always used to run.
@Zerpet
Copy link
Member Author

Zerpet commented Sep 29, 2025

@Gsantomaggio I opened an intermediate PR because the refactoring and small test changes were already accumulating quite some code. I'm continuing with the topology recovery in a different branch.

@Zerpet Zerpet requested a review from Gsantomaggio September 29, 2025 15:57
@Gsantomaggio Gsantomaggio self-assigned this Sep 29, 2025
@Gsantomaggio Gsantomaggio added this to the 0.3.0 milestone Sep 29, 2025
@Gsantomaggio Gsantomaggio added enhancement New feature or request breaking change Introduces a breaking change labels Sep 29, 2025
Copy link
Member

@Gsantomaggio Gsantomaggio left a comment

Choose a reason for hiding this comment

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

Ok with me, much better.

Added the breaking change due to TExchangeType and TQueueType, even though they are both strings.

btw we are still in 0.x so it is ok

Thank you

@Gsantomaggio Gsantomaggio merged commit 4e73e82 into main Sep 30, 2025
3 of 6 checks passed
@Zerpet Zerpet deleted the refactor-fixes branch September 30, 2025 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change Introduces a breaking change enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants