UPDATE test cases from problem specifications for prime-factors

[JagritGumber]

• Updated all the test cases including new ones added
• Regenerated test.toml with configlet
• Configlet test says all filepaths, metadata, and tests for prime-factors are up to date

[github-actions]

Hello. Thanks for opening a PR on Exercism 🙂

We ask that all changes to Exercism are discussed on our Community Forum before being opened on GitHub. To enforce this, we automatically close all PRs that are submitted. That doesn't mean your PR is rejected but that we want the initial discussion about it to happen on our forum where a wide range of key contributors across the Exercism ecosystem can weigh in.

You can use this link to copy this into a new topic on the forum. If we decide the PR is appropriate, we'll reopen it and continue with it, so please don't delete your local branch.

If you're interested in learning more about this auto-responder, please read this blog post.

---

Note: If this PR has been pre-approved, please link back to this PR on the forum thread and a maintainer or staff member will reopen it.

[github-actions]

This PR touches files which potentially affect the outcome of the tests of an exercise. This will cause all students' solutions to affected exercises to be re-tested.

If this PR does not affect the result of the test (or, for example, adds an edge case that is not worth rerunning all tests for), please add the following to the merge-commit message which will stops student's tests from re-running. Please copy-paste to avoid typos.

[no important files changed]

For more information, refer to the documentation. If you are unsure whether to add the message or not, please ping @exercism/maintainers-admin in a comment. Thank you!

[JagritGumber]

Missed some semicolons and formatting will fix that right away

[JagritGumber]

The issue is specifically with the last test case, and I am unsure as per what to do with this, because the number is a long and the test cases need to be int from my understanding

The clang fails up because the number ends up to be some negative number

[vaeng]

According to the test.toml, the "large prime" test is not new. That might have been a mistake, as you are the one who included the test just now.

I see three options:

1. skipping the test, as it does not fit the current status quo. The old example solution is for int type, thus, it would not allow such a big number. This has the advantage that old solutions are still valid.
2. adding the test, but changing the number to something below 2^31. This would be a "big number", but I am not sure it is in the spirit of the exercise.
3. changing the example solution from int to long. This would allow us to keep to the exercise spec, but old solutions might become invalid.

I prefer option 3. Number 2 is "more of the same" as the other test.

Do you have an opinion @siebenschlaefer@ErikSchierboom?

[siebenschlaefer]

The issue is specifically with the last test case, and I am unsure as per what to do with this, because the number is a long and the test cases need to be int from my understanding.

I don't think that int is required by the instructions or the tests.
To be able to implement this test with the 37-bit number 93819012551 we could change to long long or std::int64_t.
But the real problem is that it would invalidate all existing solutions when we change the expected result from std::vector<int> to std::vector<long long>.

I see three options: [...]

I agree.
I think option three is the best one in the long run because we can implement all tests without deviating from the problem specification.
If we don't want to invalidate old solutions I would vote for option two with something like 11 * 13901 * 14033 == 2145800063 because that's at least somewhat large.

[vaeng]

If we don't want to invalidate old solutions

That would touch ~1300 solutions. I could live with that, the fix is easy for everyone who understands the widening from int to long long. So if they want to fix it, it would not cost them a lot of time.

[JagritGumber]

Well if it is good with long long, I can work on updating the test cases to rather support long long instead of int, Because adding more test cases in this problem and we would encounter this problem sooner or later.

[vaeng]

Well if it is good with long long, I can work on updating the test cases to rather support long long instead of int, Because adding more test cases in this problem and we would encounter this problem sooner or later.

You would also need to update the source and header files accordingly in the .meta directory. Is that okay?

[JagritGumber]

Well if it is good with long long, I can work on updating the test cases to rather support long long instead of int, Because adding more test cases in this problem and we would encounter this problem sooner or later.

You would also need to update the source and header files accordingly in the .meta directory. Is that okay?

Yup, seems good! I see the examples use int too, I'll update that. I think it could work as a clear example of what's been changed.

[ErikSchierboom]

I'm fine with option 3, just as long as the PR contains [no important files changed] in the commit message to prevent re-testing of all the existing solutions. See https://exercism.org/docs/building/tracks#h-avoiding-triggering-unnecessary-test-runs

[vaeng]

Good work. Thanks a lot. 🚀