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 bats test for map #3923

Merged
merged 1 commit into from
Jan 31, 2024
Merged

Conversation

nicolasburtey
Copy link
Member

No description provided.

Copy link
Contributor

@vindard vindard left a comment

Choose a reason for hiding this comment

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

Two small nits but otherwise looks good

HYDRA_ADMIN_API="http://localhost:4445"


@test "admin: can retrieve admin token" {
Copy link
Contributor

Choose a reason for hiding this comment

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

We already have a test for this so it can probably just be part of the setup_file or abstracted as a function

Copy link
Member Author

Choose a reason for hiding this comment

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

yup I just copy and paste it. which option do you think is better? between moving to setup_file but where the code would be a test elsewhere (would be more obvious if something break), or having both place where this code run as part of a setup (less code duplicate, not obvious this is a test)?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can maybe treat it similar to how we have login_user in user.bash. Would mean not having an explicit test and simply having a function that has assertions within it

bats/core/api/merchant.bats Show resolved Hide resolved
@nicolasburtey nicolasburtey force-pushed the test--add-bats-test-for-map branch from 25b82d3 to 4256837 Compare January 31, 2024 20:25
@nicolasburtey nicolasburtey merged commit ee42e71 into main Jan 31, 2024
14 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.

2 participants