Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Bob_test.sh tests need fixing #178

Closed
guygastineau opened this issue Jul 18, 2018 · 12 comments · Fixed by #184
Closed

Bob_test.sh tests need fixing #178

guygastineau opened this issue Jul 18, 2018 · 12 comments · Fixed by #184

Comments

@guygastineau
Copy link
Contributor

  • The shouting with umlauts test is broken.
  • Also, they aren't umlauts.
  • Also, other tracks don't seem to have these two tests.

Requesting permission to remove these two tests in a PR

@guygastineau
Copy link
Contributor Author

Additionally, the forceful question test expects "Whoa, chill out!" when it should be "Calm down, I know what I'm doing!"

@guygastineau guygastineau changed the title Bob_test.sh shouting with umlauts is broken Bob_test.sh tests need fixing Jul 18, 2018
@budmc29
Copy link
Member

budmc29 commented Jul 19, 2018

Thanks Guy, you don't need to ask permission to open a PR, just do it and we'll discuss about it.
Opening an issue to discuss first is still good, and potentially can save you time.

Have a look at my comment here about keeping the exercise up to the problem-specification: #177 (comment)

If you see something that does not appear correct, first of all make sure that our exercise is up to date with the problem-specification track, if that's the case then you should open a PR there to fix tests (unless the test that is broken was specifically added for bash).

@budmc29
Copy link
Member

budmc29 commented Jul 19, 2018

Can you explain your first two points? We need more details.

@sjwarner-bp
Copy link
Contributor

I think that this has been discussed in the problem-specifications track to some extent? There were a couple of issues on certain platforms around the useage of non-ascii characters: exercism/problem-specifications#441

This issue boils down to the fact that this test is not up to date. The entire test suite should be brought up to date with the canonical data (over here https://github.com/exercism/problem-specifications/blob/master/exercises/bob/canonical-data.json)

@guygastineau
Copy link
Contributor Author

Thanks guys for your responses. I will update the Bob problem to the most current problem-specifications data.

@guygastineau
Copy link
Contributor Author

I have fixed bob.sh, I believe but I accidentally merged my branch for #177 into my fork master so making a PR for my fix-bob.sh branch tried to submit my commits found there too. I'll open the PR for this when my other PR goes through

@budmc29
Copy link
Member

budmc29 commented Jul 19, 2018

#177 merged.

@guygastineau
Copy link
Contributor Author

thanks @budmc29

@budmc29
Copy link
Member

budmc29 commented Jul 20, 2018

Can you please answer my question about your first two points?
#178 (comment)

@guygastineau
Copy link
Contributor Author

Okay,

  • a test can be made either to pass shouting with umlauts or pass calmy speaking with umlauts.
  • the characters in that part of the test are not actually umlauts. In German an umlaut operates on vowels by representing a simple dipthong.
  • while umlaut basically just means sound change, traditionally it is only used to reference two dots above a letter that can be upper or lower case.
    • the symbols used in those two tests looked like little capital A's, which don't seem to have a case.

@guygastineau
Copy link
Contributor Author

Opened #184 just now.
I didn't want to open a new PR for it but I managed to get my fork of exercism/bash fubar.
Everything should be able to move forward now.

Also @budmc29 Is everything regarding the unlaut tests clear, now?

@budmc29
Copy link
Member

budmc29 commented Jul 21, 2018

I don't know enough to argue if those are umlauts or not, but if they cause issues then it's better to remove them since they are not in the canonical data.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants