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 test for invalid_tag #110

Merged
merged 3 commits into from
Feb 15, 2024
Merged

add test for invalid_tag #110

merged 3 commits into from
Feb 15, 2024

Conversation

pmgreen
Copy link
Collaborator

@pmgreen pmgreen commented Feb 14, 2024

Closes #69

@pmgreen pmgreen requested a review from Beck-Davis February 14, 2024 20:18
Copy link
Contributor

@Beck-Davis Beck-Davis left a comment

Choose a reason for hiding this comment

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

Separate methods should have separate test blocks under the RSpec.describe. We should avoid nesting tests in unrelated methods (they don't call on each other).

@pmgreen
Copy link
Collaborator Author

pmgreen commented Feb 14, 2024

Ok, I wasn't sure whether validate_marc was the method in record_level.rb or whether it was the logical structure for tests.

@Beck-Davis
Copy link
Contributor

I double checked, it looks like we've been creating separate files for each method to be tested, even if the method is smaller, unless they are directly related or one calls the other. So for this one, it can have its own file.

@pmgreen
Copy link
Collaborator Author

pmgreen commented Feb 14, 2024

The invalid_tag? method is currently only checking for non-numeric characters within a tag (not length, etc.). Maybe rename it to non_numeric_tag?.

Copy link
Contributor

@Beck-Davis Beck-Davis 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

Copy link
Contributor

@Beck-Davis Beck-Davis 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

@Beck-Davis Beck-Davis marked this pull request as ready for review February 15, 2024 14:23
@Beck-Davis Beck-Davis merged commit 55047d5 into main Feb 15, 2024
2 checks passed
@Beck-Davis Beck-Davis deleted the invalid_tag_test branch February 15, 2024 14:24
@Beck-Davis
Copy link
Contributor

The invalid_tag? method is currently only checking for non-numeric characters within a tag (not length, etc.). Maybe rename it to non_numeric_tag?.

True. We can make a ticket for it.

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.

Test invalid_tag? in record_level.rb
2 participants