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

Add entity/entity alias primitives #41

Merged
merged 12 commits into from
Oct 3, 2024

Conversation

GeorgeJahad
Copy link
Collaborator

I noticed the flaky tests when running rails test, but they appear to be fixed by this PR: #40 , so I didn't address them.

@GeorgeJahad
Copy link
Collaborator Author

GeorgeJahad commented Oct 2, 2024

@suprjinx This is my first Ruby PR, still not sure of all the idioms, so don't hesitate to be harsh, when reviewing. Thanks!

dave-gantenbein
dave-gantenbein previously approved these changes Oct 2, 2024
err = assert_raises RuntimeError do
@client.read_entity_alias(@entity_name, @alias_name)
end
assert_match /no such entity/, err.message
Copy link
Collaborator

Choose a reason for hiding this comment

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

nice

auth_method = "token"
@client.put_entity_alias(@entity_name, @alias_name, auth_method)
entity_alias = @client.read_entity_alias(@entity_name, @alias_name)
assert_equal entity_alias.data[:mount_type], auth_method
Copy link
Collaborator

Choose a reason for hiding this comment

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

i think in general the first argument is the expectation, and the second is the actual being tested -- reversed here and in other test above.
https://ruby-doc.org/stdlib-3.0.0/libdoc/minitest/rdoc/Minitest/Assertions.html

@GeorgeJahad
Copy link
Collaborator Author

@suprjinx I've fixed for your comments and the tests are now passing.

Copy link
Collaborator

@suprjinx suprjinx left a comment

Choose a reason for hiding this comment

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

Looks good!

@GeorgeJahad GeorgeJahad merged commit 0f6beb1 into G-Research:main Oct 3, 2024
2 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.

3 participants