Updating tests for armstrong-numbers #2577

jagdish-15 · 2025-01-04T13:13:37Z

Pull Request

This PR adds two new test cases to the Armstrong-Numbers exercise to align it with the problem-specifications repository. However, both numbers in these new tests are extremely large and will cause the test-runner to fail. Please let me know how you'd like me to proceed with this!

github-actions · 2025-01-04T13:13:51Z

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.

SleeplessByte · 2025-01-04T16:31:12Z

exercises/practice/armstrong-numbers/armstrong-numbers.spec.js

@@ -37,4 +37,12 @@ describe('Armstrong Numbers', () => {
  xtest('Seven digit number that is not an Armstrong number', () => {
    expect(isArmstrongNumber(9926314)).toEqual(false);
  });
+
+  xtest('Armstrong number containing seven zeroes', () => {


What's the runtime for these two for the proof.ci solution on your machine?

FYI the tests are failing because that's not a valid integer. You probably need to switch to BigInt for these tests.

Yep, I’m aware of that, and that’s why comments are included in the test cases’ canonical-data.json file in the problem-specifications repository, indicating that these tests can be skipped if appropriate.

{ "uuid": "5ee2fdf8-334e-4a46-bb8d-e5c19c02c148", "description": "Armstrong number containing seven zeroes", "comments": [ "The numeric size of this input number is 108 bits. Consider skipping this test if appropriate." ], "scenarios": ["big-integer"], "property": "isArmstrongNumber", "input": { "number": 186709961001538790100634132976990 }, "expected": true }, { "uuid": "12ffbf10-307a-434e-b4ad-c925680e1dd4", "description": "The largest and last Armstrong number", "comments": [ "The numeric size of this input number is 127 bits. Consider skipping this test if appropriate." ], "scenarios": ["big-integer"], "property": "isArmstrongNumber", "input": { "number": 115132219018763992565095597973971522401 }, "expected": true }

I was wondering if we should include these tests or not because doing so might cause all community solutions to become outdated and would require new students to use BigInt. This is the concern I was referring to in my first message—apologies for not being clearer earlier! Let me know your thoughts.

Well, updating the tests causes all solutions to be rerun again so they will become outdated anyway. You are correct that the current solutions that don't use BigInt will be invalid (if we include the two new tests).

I feel that if there's an easy way to implement a solution that would pass all the tests, including the new ones, then they should be included, so if you can figure it out, feel free to commit.

I've got it working using the BigInt approach, but we can't directly pass those large numbers as they are in the test file. Instead, we need to pass them either as strings or as BigInt values. I've demonstrated both approaches in the second-to-last and last tests, respectively. Please review them and let me know which approach we should proceed with.

We barely have any BigInt exercises, so let's just upgrade everything.

I prefer it if you pass the numbers into the tests with the n suffix:

const largeNumber = 186709961001538790100634132976990n expect(isArmstrongNumber(largeNumber)).toEqual(true);

I would also recommend to add an appends file to note that some of the tests will pass a BigInt. Your proof will keep working because:

const bigInput = BigInt(42); // 42n const bigInput = BigInt(42n); // 42n

Cool-Katt

Looks good to me, and it passes the tests!

jagdish-15 · 2025-01-06T09:34:10Z

@Cool-Katt, the last two tests are not consistent yet, as they're currently taking different approaches. Let me know which approach you'd prefer me to implement for both.

Cool-Katt · 2025-01-06T09:40:03Z

I'm thinking that the one that explicitly passes a BigInt as input is better, since we want the tests to serve as a sort of hint for people that get stuck. @jagdish-15

jagdish-15 · 2025-01-06T09:46:58Z

Sure @Cool-Katt, I'll make the changes ASAP!

SleeplessByte · 2025-01-07T03:39:15Z

exercises/practice/armstrong-numbers/.meta/tests.toml

+description = "Single-digit numbers are Armstrong numbers"

 [2d6db9dc-5bf8-4976-a90b-b2c2b9feba60]
-description = "There are no 2 digit Armstrong numbers"
+description = "There are no two-digit Armstrong numbers"

 [509c087f-e327-4113-a7d2-26a4e9d18283]
-description = "Three digit number that is an Armstrong number"
+description = "Three-digit number that is an Armstrong number"

 [7154547d-c2ce-468d-b214-4cb953b870cf]
-description = "Three digit number that is not an Armstrong number"
+description = "Three-digit number that is not an Armstrong number"

 [6bac5b7b-42e9-4ecb-a8b0-4832229aa103]
-description = "Four digit number that is an Armstrong number"
+description = "Four-digit number that is an Armstrong number"

 [eed4b331-af80-45b5-a80b-19c9ea444b2e]
-description = "Four digit number that is not an Armstrong number"
+description = "Four-digit number that is not an Armstrong number"

 [f971ced7-8d68-4758-aea1-d4194900b864]
-description = "Seven digit number that is an Armstrong number"
+description = "Seven-digit number that is an Armstrong number"

 [7ee45d52-5d35-4fbd-b6f1-5c8cd8a67f18]
-description = "Seven digit number that is not an Armstrong number"
+description = "Seven-digit number that is not an Armstrong number"


Other test descriptions needs to be updated to match this.

SleeplessByte · 2025-01-07T03:40:13Z

exercises/practice/armstrong-numbers/armstrong-numbers.spec.js

@@ -37,4 +37,12 @@ describe('Armstrong Numbers', () => {
  xtest('Seven digit number that is not an Armstrong number', () => {
    expect(isArmstrongNumber(9926314)).toEqual(false);
  });
+
+  xtest('Armstrong number containing seven zeroes', () => {


We barely have any BigInt exercises, so let's just upgrade everything.

I prefer it if you pass the numbers into the tests with the n suffix:

const largeNumber = 186709961001538790100634132976990n expect(isArmstrongNumber(largeNumber)).toEqual(true);

SleeplessByte · 2025-01-07T03:40:22Z

exercises/practice/armstrong-numbers/armstrong-numbers.spec.js

+
+  xtest('The largest and last Armstrong number', () => {
+    expect(
+      isArmstrongNumber(`115132219018763992565095597973971522401`),


Suggested change

isArmstrongNumber(`115132219018763992565095597973971522401`),

isArmstrongNumber(115132219018763992565095597973971522401n),

SleeplessByte · 2025-01-07T03:41:38Z

exercises/practice/armstrong-numbers/armstrong-numbers.spec.js

@@ -37,4 +37,12 @@ describe('Armstrong Numbers', () => {
  xtest('Seven digit number that is not an Armstrong number', () => {
    expect(isArmstrongNumber(9926314)).toEqual(false);
  });
+
+  xtest('Armstrong number containing seven zeroes', () => {


I would also recommend to add an appends file to note that some of the tests will pass a BigInt. Your proof will keep working because:

const bigInput = BigInt(42); // 42n const bigInput = BigInt(42n); // 42n

jagdish-15 · 2025-01-07T15:39:06Z

@SleeplessByte, I've made all the changes you requested. However, could you please clarify what you meant by adding an append file to the note? Also, let me know if there’s anything else you'd like me to modify! Looking forward to your feedback!

SleeplessByte · 2025-01-07T15:57:51Z

Certainly

This exercise is synced with the problem-specifications, so when a change is introduced there, we receive it via pull request.

This will replace the instructions.md as expected.

We now want to add a note to that file that the solution needs to be able to handle BigNum numbers. We cannot add it to the instructions because it would eventually be overwritten.

However:

.docs/instructions.append.md: additional introduction text to append after the existing instructions (optional)

Edit or create this file for this exercise and it will remain in place when the main instructions are updated.

jagdish-15 · 2025-01-07T16:59:34Z

Sure, I'll make the changes as soon as possible! Additionally, I had a question unrelated to the PR, so I’ve sent an email to the address listed on your GitHub profile since GitHub doesn’t support direct messaging. I’d really appreciate it if you could take a look. Thank you!

Updating tests for armstrong-numbers

8b83b59

github-actions bot closed this Jan 4, 2025

SleeplessByte reopened this Jan 4, 2025

SleeplessByte reviewed Jan 4, 2025

View reviewed changes

Implementing BigInt to handle last two test cases

8fc55a0

Cool-Katt approved these changes Jan 6, 2025

View reviewed changes

SleeplessByte requested changes Jan 7, 2025

View reviewed changes

Updating test file to implement BigInt

8591449

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updating tests for armstrong-numbers #2577

Updating tests for armstrong-numbers #2577

jagdish-15 commented Jan 4, 2025

github-actions bot commented Jan 4, 2025

SleeplessByte Jan 4, 2025

SleeplessByte Jan 4, 2025

jagdish-15 Jan 4, 2025

Cool-Katt Jan 6, 2025 •

edited

Loading

jagdish-15 Jan 6, 2025

SleeplessByte Jan 7, 2025

SleeplessByte Jan 7, 2025

Cool-Katt left a comment

jagdish-15 commented Jan 6, 2025 •

edited

Loading

Cool-Katt commented Jan 6, 2025

jagdish-15 commented Jan 6, 2025

SleeplessByte Jan 7, 2025

SleeplessByte Jan 7, 2025

SleeplessByte Jan 7, 2025

SleeplessByte Jan 7, 2025

jagdish-15 commented Jan 7, 2025

SleeplessByte commented Jan 7, 2025

jagdish-15 commented Jan 7, 2025

	isArmstrongNumber(`115132219018763992565095597973971522401`),
	isArmstrongNumber(115132219018763992565095597973971522401n),

Updating tests for armstrong-numbers #2577

Are you sure you want to change the base?

Updating tests for armstrong-numbers #2577

Conversation

jagdish-15 commented Jan 4, 2025

Pull Request

github-actions bot commented Jan 4, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Cool-Katt Jan 6, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Cool-Katt left a comment

Choose a reason for hiding this comment

jagdish-15 commented Jan 6, 2025 • edited Loading

Cool-Katt commented Jan 6, 2025

jagdish-15 commented Jan 6, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jagdish-15 commented Jan 7, 2025

SleeplessByte commented Jan 7, 2025

jagdish-15 commented Jan 7, 2025

Cool-Katt Jan 6, 2025 •

edited

Loading

jagdish-15 commented Jan 6, 2025 •

edited

Loading