Some PHP katas should be modified to avoid a simple hole

[Chrono79]

While it's not a PHP issue, the language behaves different than other languages when doing implicit conversion, so when the expected result is a number, and the test suite uses assertEquals, the numbers are converted to booleans instead of the other way around, so with luck, you could pass all tests simply returning true. Sometimes the output of the function is explicitly typed, but removing that from the user's solution, makes this work. This is a list of katas already fixed (changing assertEquals for assertSame):

https://www.codewars.com/kata/find-the-stray-number https://www.codewars.com/kata/playing-with-digits https://www.codewars.com/kata/points-in-the-circle https://www.codewars.com/kata/smallest-possible-sum https://www.codewars.com/kata/evaluate-mathematical-expression https://www.codewars.com/kata/calculate-the-area-of-a-regular-n-sides-polygon-inside-a-circle-of-radius-r https://www.codewars.com/kata/financing-plan-on-planet-xy140z-n https://www.codewars.com/kata/number-zoo-patrol https://www.codewars.com/kata/5a512f6a80eba857280000fc

If you know of any other kata vulnerable to this, just list it in a reply and I'll try to fix it or warn the author/translator to fix it him/herself.

Thanks.

[Chrono79]

Needs fixing: None that I know of now.

[kazk]

Closing because there's no more left. Feel free to comment if there's more. Also, consider making a wiki page if the number is high.

[igorkharchenko]

https://www.codewars.com/kata/form-the-largest this task is also vulerable: this code works even for tests on 100 random values i.e. this code unexpectedly passes all the tests because there is no declare(strict=1) or type hinting of return type:

function maxNumber($n) {
  return true;
}

[kazk]

@IgorKharchenko in general, please use kata discussion page for kata issues and let the author know. GitHub issues are not for kata issues. If there's many kata with the problem, please let me know so I can help with organizing the information for the community to contribute.

[Chrono79]

@IgorKharchenko that particular kata was already fixed some hours ago, before you posted. Try clicking reset to see the changes.

[Chrono79]

I reopen this thread again because some more katas with this problem were found.

[Chrono79]

@kazk could you run a query to get a list of PHP katas that use assertEquals to check them?

[kazk]

Ugh, we have 824 kata matching this->assertEquals.

Is there a reason to use assertEquals? I can replace with assertSame and test it before saving the change.

[Chrono79]

I have my doubts about objects, if they're primitives, then assertSame is the way to go. Could we have a list of those katas?

Or.... we could replace and then fix only those that become broken.

[kazk]

These failed to update:

1. A Rule of Divisibility by 7
2. Area of a Square
3. Area of an arrow
4. Binary Genetic Algorithms
5. Brainfuck Translator
6. Buying a car
7. Calculate Price Excluding VAT
8. Calculate average
9. Calculate mean and concatenate string
10. Closest and Smallest
11. Convert a String to a Number!
12. Creating a custom PHP stream wrapper
13. Disease Spread
14. Driving School Series #2
15. Easy Cyclist's Training
16. Easy Line
17. Euclidean distance in n dimensions
18. Fibonacci, Tribonacci and friends
19. Financing Plan on Planet XY140Z-n
20. Find the Middle of the Product
21. Find the area of the rectangle!
22. FizzBuzz++
23. Fun with lists: filter
24. Fun with lists: map
25. Functions of Integers on Cartesian Plane
26. Heron's formula
27. Holiday VIII - Duty Free
28. How good are you really?
29. How many dots are covered
30. League Player Rank
31. Looking for a benefactor
32. MAC Address : Regexp
33. Molecule to atoms
34. Mutual Recursion
35. One Line Task: Largest Rectangle
36. Ordered Count of Characters
37. Parse a linked list from a string
38. Pascal's Triangle
39. Pattern Generator (mirrored)
40. Program a Calculator #2 - 3D Vectors
41. Quarter of the year
42. Regex Tic Tac Toe Win Checker
43. Regexp Basics - is it IPv4 address?
44. Return Negative
45. Return the closest number multiple of 10
46. Return the first M multiples of N
47. Reverse polish notation calculator
48. Screen Locking Patterns
49. Send in the Clones
50. Simple Fun #181: Rounding
51. Simple Fun #74: Growing Plant
52. Simple Fun #87: Shuffled Array
53. Square Every Digit
54. Square Pi's
55. Sum and Length
56. Sum of Array Averages
57. Sum of Intervals
58. Sum of positive
59. Super Duper Easy
60. The Walker
61. Tiny Three-Pass Compiler
62. Tortoise racing
63. Volume of a Cuboid
64. Wave Sorting
65. We are Family
66. simple calculator

[Chrono79]

I've found a problem, for example in this kata the function is typed: Sum Arrays when comparing against 0 it failed because the current solutions returned a float. I've already changed that single test to use assertEquals.

But in this other kata: Calculate average there are mixed results, some arrays contain integers and some other floats, the function isn't typed so the type is inferred. Should we change the fixed tests to comply?

[hobovsky]

Would it be possible to fix the translation in a way so it does not expect mixed output types anymore? Translation with mixed input and output types are a PITA anyway and in many cases they are just an unnecessary nuisance rather than an actual value.

[Chrono79]

I could make all the tests expect a float and add the type to the initial code too to make it clear. But this would render invalid a lot of solutions, is that ok? But, there is also another problem, it says "empty array should return 0", but it should be 0.0 if a float is expected, now it's not even tested.

Ok, for Calculate average, added the function's type, changed tests to expect a float and finally added the missing empty array test both to sample and attempt tests.