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 unit test for pop new pallet #84

Merged
merged 7 commits into from
Apr 1, 2024

Conversation

brunopgalvao
Copy link
Contributor

This PR adds a unit test for create_pallet_template.

@brunopgalvao brunopgalvao requested a review from AlexD10S March 25, 2024 13:35
Copy link
Collaborator

@AlexD10S AlexD10S left a comment

Choose a reason for hiding this comment

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

The test is good, but I feel we only testing the "happy path", can we have a test with wrong path or other alternatives that make it fail.

Also some functions used by this method in helpers like resolve_pallet_path might need specific unit test to it.

@brunopgalvao
Copy link
Contributor Author

The test is good, but I feel we only testing the "happy path", can we have a test with wrong path or other alternatives that make it fail.

Also some functions used by this method in helpers like resolve_pallet_path might need specific unit test to it.

Okay. Added some additional tests. Please approve :)

Copy link
Contributor

@al3mart al3mart left a comment

Choose a reason for hiding this comment

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

The function tested uses resolve_pallet_path function that is feature-gated with the feature parachains.

This one is set as default, but I just noticed that this is the only helper function behind a feature, maybe it is worth removing that ?

We are also including helpers in main if contracts or parachains (both default features) are active.

Aside from that nit, I guess there could be more cases covered by tests like creating a pallet within a workspace, also from different folders in it, or creating a pallet in a different path than current working directory.

Approving, though would be cool getting more cases covered.

@brunopgalvao brunopgalvao merged commit 104dfe0 into main Apr 1, 2024
1 check passed
@brunopgalvao brunopgalvao deleted the bruno/test-pop-new-pallet branch April 1, 2024 03:38
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.

3 participants