Skip to content

Minor crash resolved#473

Open
ioskunal wants to merge 1 commit intoEsqarrouth:masterfrom
ioskunal:master
Open

Minor crash resolved#473
ioskunal wants to merge 1 commit intoEsqarrouth:masterfrom
ioskunal:master

Conversation

@ioskunal
Copy link
Copy Markdown

invalid check mistake resolved

Checklist

  • New Extension
  • New Test
  • Changed more than one extension, but all changes are related
  • Trivial change (doesn't require changelog)

invalid check mistake resolved
@Esqarrouth
Copy link
Copy Markdown
Owner

Esqarrouth commented Mar 22, 2018

Thanks for the PR. Is there an associated test with this? Did it run before? Does it run now?

@ioskunal
Copy link
Copy Markdown
Author

ioskunal commented Mar 22, 2018

Thanks for quick reply. I tested this manually. The loop has to be executed from 0 to n-1 where as it runs from 0 to n and then it crashes at n index. I had checked this and now it works fine.

@EZSwiftExtensionsBot
Copy link
Copy Markdown

1 Error
🚫 Making non-trivial change requires changelog entry! Please, set trivial change or add entry to changelog.
2 Messages
📖 Executed 203 tests, with 0 failures (0 unexpected) in 6.527 (6.789) seconds
📖 Executed 125 tests, with 0 failures (0 unexpected) in 4.777 (4.869) seconds

Generated by 🚫 Danger

@Esqarrouth
Copy link
Copy Markdown
Owner

Could you edit the test cases here so it gives an asset while running the code:

for i in 0...length {

But doesn't give an assert with:

for i in 0..<length {

https://github.com/goktugyil/EZSwiftExtensions/blob/master/EZSwiftExtensionsTests/StringTests.swift

/// EZSE: Checks if String contains Emoji
public func includesEmoji() -> Bool {
for i in 0...length {
for i in 0..<length {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could you write a unit test for this pls ?

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.

4 participants