ruby
ruby copied to clipboard
Add tests to meetup exercise to test ":fifth".
If these are acceptable, I can start adding them to meetup in other languages.
Check https://github.com/exercism/problem-specifications in case the tests needs to be added there. Some languages use generators to create the tests from the specifications there.
I know that the work is done here, but the special areas to test for the fifth is months that only have 4 weeks (February, not a leap year). And then a good case, such as any other month that has 5 weeks, with the date that is there and then a case where there is a fifth week, but not a fifth occurrence of the day.
So maybe 4 tests total to cover the potentials?
The tests are indeed missing from the problem specifications, so it looks like I should add them there -- once I understand your feedback. Thanks.
I have a couple of questions about your feedback:
- I added tests to mirror the pattern I saw with every other descriptor. They each have two tests per weekday. Are you saying that this PR should only add 4 tests total? That seems lopsided.
- You mention nonsensical input such as the fifth Monday in a month where there isn't a fifth Monday. None of the existing tests cover nonsensical input, so I didn't include any, either.
Also, the instructions don't specify what should be returned on nonsensical input, so I'm not sure what such a test ought to establish.
Please let me know. I'm happy to revise as necessary.
The tests are indeed missing from the problem specifications, so it looks like I should add them there -- once I understand your feedback. Thanks.
I have a couple of questions about your feedback:
- I added tests to mirror the pattern I saw with every other descriptor. They each have two tests per weekday. Are you saying that this PR should only add 4 tests total? That seems lopsided.
Perhaps it is lopsided, but it may be lopsided in that the tests that are already existent are not fully necessary. Might consider it (and I will defer to your experience and impression as you are much closer to that code and the exercise intention at the moment than I am). Having more tests than necessary, redundant tests, are really not helpful, generally speaking.
- You mention nonsensical input such as the fifth Monday in a month where there isn't a fifth Monday. None of the existing tests cover nonsensical input, so I didn't include any, either.
Asking for the fifth Saturday in February 2020 is possible, and will be found. Asking for the fifth Friday in February 2020 is not nonsensical, because it could exist, but happens that it does not for that year. Still, if we have not verified on a calendar that it exists, then we should not ask the program? But maybe it is out of scope. Maybe an area for discussion in problem-specifications
Also, the instructions don't specify what should be returned on nonsensical input, so I'm not sure what such a test ought to establish.
Fair enough. Though asking for the fifth Saturday in July is sometimes nonsensical, and sometimes makes sense to do so. Asking for it in year 2020 is then, I suppose, nonsense. But how would we know without looking at a calendar, or perhaps using the program to know?
If the instructions do not cover what to do, then that is at the discretion of the solver.
Please let me know. I'm happy to revise as necessary.
Yep. No problems, thanks for asking for clarification. I pasted a calendar below for reference, for the discussed year.
2020
January February March
Su Mo Tu We Th Fr Sa Su Mo Tu We Th Fr Sa Su Mo Tu We Th Fr Sa
1 2 3 4 1 1 2 3 4 5 6 7
5 6 7 8 9 10 11 2 3 4 5 6 7 8 8 9 10 11 12 13 14
12 13 14 15 16 17 18 9 10 11 12 13 14 15 15 16 17 18 19 20 21
19 20 21 22 23 24 25 16 17 18 19 20 21 22 22 23 24 25 26 27 28
26 27 28 29 30 31 23 24 25 26 27 28 29 29 30 31
April May June
Su Mo Tu We Th Fr Sa Su Mo Tu We Th Fr Sa Su Mo Tu We Th Fr Sa
1 2 3 4 1 2 1 2 3 4 5 6
5 6 7 8 9 10 11 3 4 5 6 7 8 9 7 8 9 10 11 12 13
12 13 14 15 16 17 18 10 11 12 13 14 15 16 14 15 16 17 18 19 20
19 20 21 22 23 24 25 17 18 19 20 21 22 23 21 22 23 24 25 26 27
26 27 28 29 30 24 25 26 27 28 29 30 28 29 30
31
July August September
Su Mo Tu We Th Fr Sa Su Mo Tu We Th Fr Sa Su Mo Tu We Th Fr Sa
1 2 3 4 1 1 2 3 4 5
5 6 7 8 9 10 11 2 3 4 5 6 7 8 6 7 8 9 10 11 12
12 13 14 15 16 17 18 9 10 11 12 13 14 15 13 14 15 16 17 18 19
19 20 21 22 23 24 25 16 17 18 19 20 21 22 20 21 22 23 24 25 26
26 27 28 29 30 31 23 24 25 26 27 28 29 27 28 29 30
30 31
October November December
Su Mo Tu We Th Fr Sa Su Mo Tu We Th Fr Sa Su Mo Tu We Th Fr Sa
1 2 3 1 2 3 4 5 6 7 1 2 3 4 5
4 5 6 7 8 9 10 8 9 10 11 12 13 14 6 7 8 9 10 11 12
11 12 13 14 15 16 17 15 16 17 18 19 20 21 13 14 15 16 17 18 19
18 19 20 21 22 23 24 22 23 24 25 26 27 28 20 21 22 23 24 25 26
25 26 27 28 29 30 31 29 30 27 28 29 30 31
It sounds like you are suggesting that the test suite as a whole should be pruned, and the exercise itself should be expanded to cover input which is... well-formed, but for which there is no date, such as "the fifth Saturday in February 2020". (I called that input "nonsensical", but that was not an accurate term.)
I am suggesting adding to the code in a minimally invasive, consistent way, but the project would be better with your suggestions. I'll bring both these ideas to the problem-specifications repo and report back.
It sounds like you are suggesting that the test suite as a whole should be pruned, and the exercise itself should be expanded to cover input which is... well-formed, but for which there is no date, such as "the fifth Saturday in February 2020". (I called that input "nonsensical", but that was not an accurate term.)
I took no offense to the term, recognized perhaps it was quite the full meaning, but the hint was there that it might be something a bit silly to do if it never makes sense to ask for something. (We do not test for 2nd banana, after all!).
I am suggesting not that you should take on the pruning of all the tests, but only that your change specifically, should be as concise, yet complete, as possible, while still being clear.
It will, over time, mean that others are inspired to do the same if they find something to add to the tests, and may give someone else inspiration to remove what may be "fluff" from the tests.
I am suggesting adding to the code in a minimally invasive, consistent way, but the project would be better with your suggestions. I'll bring both these ideas to the problem-specifications repo and report back.
Yes, minimally invasive, but not consistent when being consistent seems not as good as the more elegant, concise, complete change that is clear.
I am not saying that the full set of tests should be full of what you called "well-formed" tests, but I am saying that your limited change, if it looks differently than the others, has only the smallest number of tests needed to express it, and optionally (and with consideration) the discussed edge cases, we will likely accept the minimal number of tests, being they are complete and consistent, without being the same number of tests as the other areas.
The danger of fully specified exercises is a potential for lost exploration and discussion during the mentoring process. I have seen this happen several times over decades, and the negative effect on overall learning, so that part should be taken seriously and carefully, if we decide to fully specify what was noted. It could also be an additional exercise, where we "noticed that we might have a query about something that does not always happen" and address it with other goals in mind.
When you are in the problem-specifications and open an issue or pull request (probably issue first, as it effects all tracks), do not forget to mention the discussion here, maybe for additional context.
@houhoulis Any luck in problem-specifications repository in regard to this issue?
@houhoulis Any luck in problem-specifications repository in regard to this issue?
Sorry to have let this topic go dormant, @kotp. I had to make a PR in problem-specifications just to understand their guidelines for contributing, and that took long enough that I didn't get back to the actual question we discussed.
I'll try to follow up with problem-specifications this weekend.
Sounds good, no rush.
@houhoulis @kotp It's been a while since anything happened here, and I'm trying to get a sense of the state of this PR.
Do you happen to remember where the discussion about adding "fifth" tests to the meetup exercise took place?
Back in the day we decided not to add tests for fifth cases because the framing of the exercise is around scheduling regular, monthly meetups. Since not all months have a fifth week (and even if there is a fifth week it might not have the requisite day) we decided to omit "fifth" from the exercise, as it would not be a good fit for a regular meetup schedule.
If that decision was revisited we'd want to update the problem-specifications to reflect the reasoning for it.
Right on, @kytrinyx , this was the same direction I was guiding @houhoulis , but do not see any reference from problem-specifications issues or PR back to here after the end of that week. So I do not think it went anywhere.
I think the exercise is OK without it, it may be in some mentor notes individually, (no way to observe mentoring any more), but nothing further for the last 2 ¼ years.
I can close it (without prejudice). It can be reopened at such time that discussion happens in exercism/problem-specifications.