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

Truncate profile name using CSS instead of rails helper #554

Merged
merged 5 commits into from
Nov 18, 2024

Conversation

voodoo
Copy link
Contributor

@voodoo voodoo commented Nov 17, 2024

This was generated with a system test. I'm not seeing how we can tests the value in that field. It does not see the ... from truncate.

failures_test_should_truncate_the_name

@voodoo
Copy link
Contributor Author

voodoo commented Nov 17, 2024

password_resets_controller_test.rb failed? I don't get it.

I dont 'think' we will be able to test truncate.

https://stackoverflow.com/questions/51630998/capybara-and-text-overflow-ellipsis

Capybara depends on the text being returned by the drivers. The drivers are trying to meet the WebDriver spec definition for what text to return, which is currently defined at https://w3c.github.io/webdriver/#get-element-text . That spec states that it is intended to return the text "as rendered", however the bot.dom.getVisibleText given as the standard doesn't currently take text-overflow into account. This means that as of now there's not much Capybara can do about it, and there really isn't a way to directly test it.

https://stackoverflow.com/questions/18090678/is-it-possible-to-call-truncate-method-in-rspec-feature-test

https://medium.com/@dave_go/testing-text-truncation-effective-strategies-2a589841a22b

@krschacht krschacht changed the title System truncate Truncate profile name using CSS instead of rails helper Nov 18, 2024
@krschacht
Copy link
Contributor

@voodoo I saw you tee'd up a separate PR simply to remove the old approach, but this whole PR is small enough I think it's fine to combine them. Also, I agree that this aspect of display cannot be easily tested with automated testing. I think it's fine that it's not covered by our test suite.

I saw a failed system test, but that appears to be a flappy test. It's really hard to make system tests 100% reliable so I simply clicked "re-run failed test" within github CI and it passed the second time. All looks good on this PR so I'll merge in!

@krschacht krschacht merged commit 7a58fe9 into AllYourBot:main Nov 18, 2024
6 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