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

Makes most strings tests pass #16

Merged
merged 2 commits into from
Sep 6, 2023
Merged

Makes most strings tests pass #16

merged 2 commits into from
Sep 6, 2023

Conversation

gampleman
Copy link
Collaborator

@ahankinson I took a stab at this.

I think you can still get failures with high enough --fuzz values, but the build should now pass at least most of the time, which I think is a significant milestone.

There are a few fixes/changes to behaviour that I think are generally sensible.

These are:

-- Before
String.Extra.softBreak 1 "    " --> ["    "]
String.Extra.softBreak 3 "foo    bar" --> ["foo    ", "bar"]

-- After
String.Extra.softBreak 1 "    " --> [" ", " ", " ", " "]
String.Extra.softBreak 3 "foo    bar" --> ["foo", "   ", " bar"]

-- Before
String.Extra.toTitleCase "č" --> "č"

-- After
String.Extra.toTitleCase "č" --> "Č"

-- Before
String.Extra.clean "foo\tbar" --> "foo\tbar"

-- After
String.Extra.clean "foo\tbar" --> "foo bar"

I also deleted a test that was just wrong 🤷.

@ahankinson
Copy link
Contributor

Looks great to me! I did manage to get it to fail with a high fuzz rate:

Running 574 tests. To reproduce these results, run: elm-test --fuzz 100000 --seed 316688162955213

↓ String.HumanizeTest
↓ humanize
✗ It removes a trailing `_id` & replaces underscores and dashes with a single whitespace

Given "__id"

    "id"
    ╷
    │ Expect.equal
    ╵
    ""

I'm not sure if that's something you think is worth fixing or not? (c.f., your slack messages about "_id")

@gampleman gampleman merged commit 9776a8e into master Sep 6, 2023
4 checks passed
@gampleman gampleman deleted the string-fixes branch September 6, 2023 08:39
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