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.

Sign up for GitHub

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

Jump to bottom

Removing the word 'error' for Lorem Ipsum generator #2663

Closed
philipfong opened this issue Dec 19, 2022 · 4 comments
Closed

Removing the word 'error' for Lorem Ipsum generator #2663

philipfong opened this issue Dec 19, 2022 · 4 comments
Labels
💡 Issue: Feature Request

Comments

@philipfong
Copy link
Contributor

philipfong commented Dec 19, 2022 •
edited
Loading

Is your feature request related to a problem? Please describe it.
On occasion, in our tests, we use lorem ipsum generation to insert text in various parts of the app. In some of these areas, we'll perform some broad/sweeping checks of some common JavaScript errors that could potentially come up (things like NaN for example). One of those keywords we check for is error.

If you're adding new objects, please describe how you would use them
Removing the word 'error' from the lorem ipsum generator could avoid false negatives in testing.

Describe alternatives you've considered
We manually perform a text replacement for the word 'error' or 'Error' in order to avoid tests from failing.

Additional context
As far as test design goes, I think it is sound to perform generic text checks like the one described, especially if the app under test has experienced some of these problems with things like NaN or null or error appearing in app or in email communications. Overall, removing 'error' from the lorem ipsum dictionary wouldn't add much downside and likely wouldn't be missed.
@stefannibrasil
Copy link
Contributor

hi @philipfong, thank you for opening this issue and for providing the details.

I don't see why not just remove the error word from the Lorem locale file.

Are you okay with opening a PR for that? As a bonus, I think this would be a great opportunity to reformat this file to use the dash syntax instead.

@psibi
Copy link
Member

psibi commented Dec 20, 2022 •
edited
Loading

I'm not sure if we should remove this, since the original Lorem ipsum does contain the word error and it wouldn't be correct generation if it omits some words. I think it's best if a new method is created which blacklists some words ?

@philipfong
Copy link
Contributor Author

I think @psibi is right. Maybe a new option in Faker::Lorem like :exclude_words that maybe takes a list and removes it from the list of Lorem words. I'll look at putting up a PR for that.

This was referenced Dec 20, 2022
Open
Merged
@philipfong
Copy link
Contributor Author

I created a pull request to the repo here: #2665

I do think the new keyword is useful while keeping it optional. I've added tests as well.

philipfong pushed a commit to philipfong/faker-issue-2663 that referenced this issue Dec 21, 2022
Fixes faker-ruby#2663 add new keyword argument that excludes lorem ip…
c70cff6 
…sum words from being produced

fix(faker-2663): remove pry

fix(faker-2663): linting

fix(faker-2663): use instance_of and self assignment

fix(faker-2663): use instance_of? for Array

feat(faker-2663): allow for comma delimited string to be passed
@psibi psibi closed this as completed in ba5cea0 Dec 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
💡 Issue: Feature Request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@psibi @stefannibrasil @philipfong