Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updating tests for armstrong-numbers #2577

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+51 −23
Open

Updating tests for armstrong-numbers #2577

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8b83b59
Updating tests for armstrong-numbers
jagdish-15 Jan 4, 2025
8fc55a0
Implementing BigInt to handle last two test cases
jagdish-15 Jan 6, 2025
8591449
Updating test file to implement BigInt
jagdish-15 Jan 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions exercises/practice/armstrong-numbers/.meta/proof.ci.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
export const isArmstrongNumber = (input) => {
const digits = [...String(input)];
const bigInput = BigInt(input);

const digits = [...String(bigInput)];

const sum = digits.reduce(
(total, current) => total + current ** digits.length,
0,
(total, current) => total + BigInt(current) ** BigInt(digits.length),
BigInt(0),
);
return sum === input;

return sum === bigInput;
};
35 changes: 24 additions & 11 deletions exercises/practice/armstrong-numbers/.meta/tests.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,30 +1,43 @@
# This is an auto-generated file. Regular comments will be removed when this
# file is regenerated. Regenerating will not touch any manually added keys,
# so comments can be added in a "comment" key.
# This is an auto-generated file.
#
# Regenerating this file via `configlet sync` will:
# - Recreate every `description` key/value pair
# - Recreate every `reimplements` key/value pair, where they exist in problem-specifications
# - Remove any `include = true` key/value pair (an omitted `include` key implies inclusion)
# - Preserve any other key/value pair
#
# As user-added comments (using the # character) will be removed when this file
# is regenerated, comments can be added via a `comment` key.

[c1ed103c-258d-45b2-be73-d8c6d9580c7b]
description = "Zero is an Armstrong number"

[579e8f03-9659-4b85-a1a2-d64350f6b17a]
description = "Single digit numbers are Armstrong numbers"
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"
Comment on lines +16 to +37
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other test descriptions needs to be updated to match this.


[5ee2fdf8-334e-4a46-bb8d-e5c19c02c148]
description = "Armstrong number containing seven zeroes"

[12ffbf10-307a-434e-b4ad-c925680e1dd4]
description = "The largest and last Armstrong number"
11 changes: 11 additions & 0 deletions exercises/practice/armstrong-numbers/armstrong-numbers.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,15 @@ describe('Armstrong Numbers', () => {
xtest('Seven digit number that is not an Armstrong number', () => {
expect(isArmstrongNumber(9926314)).toEqual(false);
});

xtest('Armstrong number containing seven zeroes', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@Cool-Katt Cool-Katt Jan 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

const largeNumber = BigInt(`186709961001538790100634132976990`);
expect(isArmstrongNumber(largeNumber)).toEqual(true);
});

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
isArmstrongNumber(`115132219018763992565095597973971522401`),
isArmstrongNumber(115132219018763992565095597973971522401n),

).toEqual(true);
});
});