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

refactor(landscape): Refactor Landscape receive-commands tests #514

Merged
merged 2 commits into from
Jan 30, 2024

Conversation

EduardGomezEscandell
Copy link
Contributor

@EduardGomezEscandell EduardGomezEscandell commented Jan 25, 2024

This PR concerns the Landscape service package tests that tested the execution of commands comming from the server. It is motivated by the attempt at adding new tests in PR #506 regarding cloud-init integration.

These tests where sub-par for two reasons:

  1. They were structured in a way that was hard to read
  2. They had very little granularity: only one success and one failure test case per command.

Adding new tests was quite hard, defeating the purpose of table testing. With this new structure, adding test cases is much simpler, so I added quite a few of them.

A side effect of this PR is that the executor tests can no longer be ran without the mock. We were never running them anyways so I decided against maitaining unused code.

These tests where bad for two reasons:
1. They were structured in a way that was hard to read
2. They had very little granularity: only one success and one failure
   test case per command.

Adding new tests was quite hard, defeating the purpose of table testing.
With this new structure, adding test cases is much simpler, so I added
quite a few of them.

This is relevant to an upcomming pull request regarding cloud-init
integration.
@EduardGomezEscandell EduardGomezEscandell self-assigned this Jan 25, 2024
@EduardGomezEscandell EduardGomezEscandell marked this pull request as ready for review January 25, 2024 12:27
@EduardGomezEscandell EduardGomezEscandell requested a review from a team as a code owner January 25, 2024 12:27
@EduardGomezEscandell EduardGomezEscandell marked this pull request as draft January 25, 2024 12:29
@EduardGomezEscandell EduardGomezEscandell marked this pull request as ready for review January 25, 2024 13:04
Copy link
Contributor

@CarlosNihelton CarlosNihelton left a comment

Choose a reason for hiding this comment

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

Looks better than before.

@EduardGomezEscandell EduardGomezEscandell merged commit 1a72e4f into main Jan 30, 2024
33 checks passed
@EduardGomezEscandell EduardGomezEscandell deleted the landscape-refactor-tests branch January 30, 2024 08:56
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