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

test: Add new unit test framework with JSON fixtures #589

Merged
merged 14 commits into from
Oct 23, 2024

Conversation

ykim-akamai
Copy link
Contributor

@ykim-akamai ykim-akamai commented Oct 9, 2024

📝 Description

This PR implements new improved unit testing framework that allows writing unit test against JSON fixtures. Here is the list of core files for the framework:

base.go:
Implements the ClientBaseCase struct for setting up the mock Linode client. This file includes utility functions like SetUp, TearDown, and MockGet to simplify mocking API requests for unit tests.

fixtures.go:
Provides the TestFixtures struct for loading and retrieving test fixtures from JSON files. This allows test cases to easily mock API responses using predefined data from the fixtures directory

instance_test.go:
Example test file containing test for List Instances endpoint via command base.Client.ListInstances(context.Background(), nil)

e.g. fixtures/linodes_list.json:
JSON fixture file containing mock data for Linode instances. This is used in the unit test to simulate a response from the List Instances API endpoint.

✔️ How to Test

make testunit

📷 Preview

If applicable, include a screenshot or code snippet of this change. Otherwise, please remove this section.

@ykim-akamai ykim-akamai requested a review from a team as a code owner October 9, 2024 17:22
@ykim-akamai ykim-akamai requested review from jriddle-linode and ezilber-akamai and removed request for a team October 9, 2024 17:22
@ykim-akamai ykim-akamai marked this pull request as draft October 9, 2024 17:22
@lgarber-akamai lgarber-akamai self-requested a review October 9, 2024 17:42
}

// loadFixtures loads all JSON files in fixtures directory
func (tf *TestFixtures) loadFixtures() {
Copy link
Contributor

@lgarber-akamai lgarber-akamai Oct 9, 2024

Choose a reason for hiding this comment

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

optional: One other potential way to handle this would be using the embed directive with the embed.FS type. I think the current implementation is perfectly acceptable too, just figured I'd share another potential approach 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Appreciate the feedback. I wasn't aware of this option at all, but it seems like a great improvement to implement. This change should enhance performance as the test framework scales with additional tests and components 👍

@ykim-akamai ykim-akamai marked this pull request as ready for review October 22, 2024 14:56
Copy link
Contributor

@lgarber-akamai lgarber-akamai left a comment

Choose a reason for hiding this comment

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

Super clean implementation and everything is working on my end. Great work!

Copy link
Contributor

@ezilber-akamai ezilber-akamai left a comment

Choose a reason for hiding this comment

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

Tested locally and everything is working gread!

@ezilber-akamai ezilber-akamai self-requested a review October 23, 2024 19:36
@ykim-akamai ykim-akamai merged commit 79809ba into linode:main Oct 23, 2024
5 checks passed
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.

4 participants