Skip to content

Conversation

@dkrutsko
Copy link
Member

No description provided.

@dkrutsko dkrutsko added the Ready to Merge The pull request has been approved for merging label Jul 17, 2023
@dkrutsko dkrutsko self-assigned this Jul 17, 2023
Copy link

@magickatt magickatt left a comment

Choose a reason for hiding this comment

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

Makes sense, although would be good to add tests to validate these cases in the future (probably more time-critical to get these out sooner rather than later)

@dkrutsko
Copy link
Member Author

dkrutsko commented Jul 19, 2023

Yeah, normally I would add unit tests but I haven't found an optimal strategy for this yet as it relies on files and the AWS API. I'm sure there's a way, I just don't want to have to mock things. Maybe it would make sense to have a special EC2 machine that is set up specifically for testing.

Though I suppose you could also test the manager functions separately, particularly those that don't rely on files or AWS. The command-line functions, however, might need to think a bit more about, I'm sure there's a way though.

@dkrutsko dkrutsko merged commit 88043de into main Jul 19, 2023
@dkrutsko dkrutsko deleted the dave/systemd branch July 19, 2023 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ready to Merge The pull request has been approved for merging

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants