exercism / java

Exercism exercises in Java.
https://exercism.org/tracks/java
MIT License
688 stars 672 forks source link

Locally installed tests don't seem to match server side tests in "Phone Number" #2340

Closed djspinmonkey closed 8 months ago

djspinmonkey commented 1 year ago

When working on the "Phone Number" exercise, I got all tests passing locally, but when submitting a solution, the test suite failed on the site with a cannot find symbol error. It looks like there's a method being called in the local tests called getPhoneNumber() which is instead called getNumber() in the server side version.

Having not dug into this issue, I'm not sure where to find the two different versions of these tests, but I was very surprised that they were different at all! I'm sure there are reasons, but my assumption would have been that the test suite provided is the same one run on the server.

As a side note (since it's probably not worth opening a ticket just for this), there were some other questionable aspects of this exercise, as well. A relatively minor point, but the error messages are all lowercase instead of being capitalized, and could generally be more clear. For example, "punctuations not permitted" (note the plural) should probably be "Punctuation not permitted", or even better, "Phone numbers may not contain punctuation other than parentheses and dashes".

More seriously in my opinion, it is very poor style to have a field named the same as the class in which it appears. In this case, the PhoneNumber class has a phoneNumber field. Either a phone number warrants its own type, in which case getPhoneNumber() should be called toString(), or it doesn't, in which case there shouldn't be any PhoneNumber class, and this exercise should be about a PhoneNumberFormatter or some such, instead.

SleeplessByte commented 1 year ago

👋🏽

I have checked the tests for you and it seems that all the way from 2014 until now, the method is really getNumber() and not getPhoneNumber(). I have compared that against a fresh start on the web editor which also shows me getNumber().

Web editor of fresh exercise java/phone-number

You may want to try this in the web editor as well. If yours doesn't look like the screenshot above, you may want to try the following option:

menu option in web editor called revert to exercise start

You also want to go to https://exercism.org/tracks/java/exercises/phone-number and ensure there is no prompt to update the exercise.

If nothing else works, can you confirm that you didn't change the tests yourself and did not submit the test file to the server? If you did not, we can ask Jeremy or Erik to check the database why it is showing different tests.


Changes in the language tests use should be considered first in the canonical problem specification so that a change may be propagated throughout all the implementations across tracks.


Yeah I agree with you on the implementation being super weird... we'll take a PR!

djspinmonkey commented 1 year ago

Thanks for the quick update!

I didn't use the web editor for this (and don't use it generally) - instead, I used the CLI tool to download the exercise and work on it locally. I downloaded it fresh yesterday, and did not change the tests myself. I also did not submit the test file to the server, only the PhoneNumber.java file. I just took double checked and took a quick screen shot - this shows the getPhoneNumber() method in the first test, but the rest of the tests called it as well. Phone Number Tests Bug

2014 is obviously quite a while - is it possible that something about the build and deploy pipeline got wedged years ago, and has been serving an old version of the exercise via the CLI since then? That seems like a very long time for something like that to persist, but clearly something happened to cause this.


FWIW, I'm not worried about finishing the exercise for myself. I was just looking for something quick to refresh my memory on basic Java syntax, since I've mostly been working in other languages lately. My goal here is to help prevent this issue from confusing other folks coming along later, especially those newer to programming.


That's interesting that there's a canonical problem specification that includes the exact error messages. I can see where it would be very valuable in some ways, but how is it recommended to handle differences in idiomatic behavior between different languages? Case in point, in Java I would expect the error messages to be full sentences with capitalization, but that might be unidiomatic in other languages.


I'd be happy to submit a PR to adjust the implementation! :smile: I think I've got some time later today to knock that out.

djspinmonkey commented 1 year ago

Ok, I've opened a PR!

https://github.com/exercism/java/pull/2341

Currently, it only addresses the naming of getNumber(), changing it to toString() (and a little minor cleanup of the description). I'd be happy to adjust the error messages and such, too, but I don't want to mess up the error messages for other language tracks in the process, and I'm not sure what the recommended way to address that is.

SleeplessByte commented 1 year ago

I should have been clearer.

The file has never had getPhoneNumber even since the first iteration in 2013. Perhaps an accidental search-and-replace has taken place? If not, we should find someone who has the same test file as you do and then try to find the pattern.

sanderploegsma commented 8 months ago

Closing this due to inactivity.