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

command rev is not always working in exercism tests #561

Closed
Kidlike opened this issue Dec 25, 2021 · 7 comments
Closed

command rev is not always working in exercism tests #561

Kidlike opened this issue Dec 25, 2021 · 7 comments
Labels

Comments

@Kidlike
Copy link
Contributor

Kidlike commented Dec 25, 2021

Hi,

I was implementing https://exercism.org/tracks/bash/exercises/luhn, and I tried to use rev to reverse the input string, so I can iterate in a positive loop (starting from 0 index).

While locally this worked fine, when I submitted it, one of the tests executed by exercism.io failed:

run bash luhn.sh "055£ 444$ 285"
assert_success
assert_output "false"
(from function `assert_output' in file bats-extra.bash, line 394,
 in test file luhn.bats, line 94)
  `assert_output "false"' failed

-- output differs --
expected (1 lines):
  false
actual (2 lines):
  rev: stdin: Invalid or incomplete multibyte or wide character
  false
--

I don't know if it's in the spirit of this track to use such commands, but I thought I should report this regardless :)

@glennj
Copy link
Contributor

glennj commented Dec 26, 2021

I'd have to see your code, and what you're feeding into rev. Can you paste a link to it, or a mentoring invitation?

As a guess, try moving the error checking earlier in your code, so that non-digits are detected before piping them into rev.

@Kidlike
Copy link
Contributor Author

Kidlike commented Dec 26, 2021

You're right, that I could use rev after validating the input; but I solved it without using rev at all, which I guess is even better in the spirit of the bash track.

Regardless, this error comes from running input=$(echo -n "${*//\ /}" | rev) as the first command in the script.

The question is more like, should this generally work or not? From some googling it looks like this problem is system-related (OS? packages? locale config? not sure...)

@IsaacG
Copy link
Member

IsaacG commented Dec 26, 2021

The exercise can (and should) be solved via bash and not other utilities. rev is part of util-linux and may or may not exist on BSD or MacOS. It may differ from distro to distro. The behavior of other utilities that may or may not exist in the test environment should, as much as possible, be out of scope of these exercises.

I could create a similar bug about how executing python <<< 'print("foo".removeprefix("f"))' might not work because the image doesn't contain Python 3.9. It's not bash` related though.

@matt-kita
Copy link

matt-kita commented Jan 12, 2022

The exercise can (and should) be solved via bash and not other utilities. rev is part of util-linux and may or may not exist on BSD or MacOS. It may differ from distro to distro. The behavior of other utilities that may or may not exist in the test environment should, as much as possible, be out of scope of these exercises.

Right. Additionally - rev works on a line-by-line basis, not on a character-by-character basis, so the newline characters ordering a.k.a. line ordering (if any newlines are present in the input) is also not reverted. The questions remain: should it be reverted as well? should it be tested somehow?

Example:

$ rev <<<'hello *   world
newline?'
dlrow   * olleh
?enilwen
$ main 'hello *   world
newline?'
?enilwen
dlrow   * olleh
$

@IsaacG
Copy link
Member

IsaacG commented Jan 12, 2022

I think a test which includes newlines would be a valuable addition here.

@glennj
Copy link
Contributor

glennj commented Jul 10, 2022

Raised in exercism/problem-specifications#2073

@glennj glennj added the wontfix label Jul 12, 2022
@glennj
Copy link
Contributor

glennj commented Jul 12, 2022

Community consensus is to put "more advanced" reverse cases into a new exercise.

Closing this issue.

@glennj glennj closed this as completed Jul 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants