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 diffie-hellman #2584

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

updating tests diffie-hellman #2584

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c1ab732
updating tests diffie-hellman
jagdish-15 Jan 5, 2025
ac1da54
Updating names in test file to match those in test.toml
jagdish-15 Jan 6, 2025
d71c63d
fixing formatting issue
jagdish-15 Jan 6, 2025
dd07216
Adding getPrivateKey implementation and tests
jagdish-15 Feb 4, 2025
7bc0075
Merge branch 'update-tests-diffie-hellman' of https://github.com/jagd…
jagdish-15 Feb 4, 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
1 change: 1 addition & 0 deletions exercises/practice/diffie-hellman/.meta/config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
],
"contributors": [
"ankorGH",
"jagdish-15",
"rchavarria",
"serixscorpio",
"SleeplessByte",
Expand Down
4 changes: 4 additions & 0 deletions exercises/practice/diffie-hellman/.meta/proof.ci.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,4 +113,8 @@ export class DiffieHellman {
PRIMES.includes(g)
);
}

static getPrivateKey(p) {
return Math.floor(Math.random() * (p - 1) + 2);
}
}
18 changes: 14 additions & 4 deletions exercises/practice/diffie-hellman/.meta/tests.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,26 @@
# 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.

[1b97bf38-4307-418e-bfd2-446ffc77588d]
description = "private key is in range 1 .. p"
description = "private key is greater than 1 and less than p"

[68b2a5f7-7755-44c3-97b2-d28d21f014a9]
description = "private key is random"

[b4161d8e-53a1-4241-ae8f-48cc86527f22]
description = "can calculate public key using private key"

[0d25f8d7-4897-4338-a033-2d3d7a9af688]
description = "can calculate public key when given a different private key"

Cool-Katt marked this conversation as resolved.
Show resolved Hide resolved
[cd02ad45-3f52-4510-99cc-5161dad948a8]
description = "can calculate secret using other party's public key"

Expand Down
4 changes: 4 additions & 0 deletions exercises/practice/diffie-hellman/diffie-hellman.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,8 @@ export class DiffieHellman {
getSecret(theirPublicKey, myPrivateKey) {
throw new Error('Remove this statement and implement this function');
}

getPrivateKey() {
throw new Error('Remove this statement and implement this function');
}
}
23 changes: 22 additions & 1 deletion exercises/practice/diffie-hellman/diffie-hellman.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ describe('diffie-hellman', () => {
}).toThrow();
});

describe('input validation', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Since our tests include custom cases that are not part of the predefined ones, the structure of our test file in not standard either. As such, the name of this section of test cases is not incorrect, and shouldn't be changed.

However, it is missing two of the standard tests, private key is greater than 1 and less than p and private key is random (i think they would be placed under validation).
@jagdish-15 can you implement the two missing tests? You can look at other tracks for inspiration, for example something similar to the Java or C# tracks' implementations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure! I'll make the changes by checking out other tracks for inspiration. It'll take a bit of time since I need to prepare for a college exam tomorrow, but I'll get to it once the exam is over. Thanks for your understanding!

Copy link
Contributor

Choose a reason for hiding this comment

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

best of luck on the exam!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you so much for the kind words, @Cool-Katt! I really appreciate it!

Regarding the tests, I realized while implementing them that they check the getPrivateKey method, which isn't included in either the student starter code or the proof solution. After reviewing other tracks, I saw that this method typically returns a private key, randomly chosen between 1 and p, exclusive (i.e., strictly between 1 and p). Since the tests are designed to validate this, it seems we should have such a method in place.

Would you like me to add the getPrivateKey method to both the proof solution and the starter code for students?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, that would probably explain why the tests for validating the private key were missing 😅
You can also amend the stub and the ci proof

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, will get to it as soon as possible!

Copy link
Member

Choose a reason for hiding this comment

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

@jagdish-15 how is it going now? Still have time for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@SleeplessByte, I apologize for the delay—I didn’t intend to hold up the PR. I was occupied with organizing a college club event for CodePVG. I’ll get to work on this right away!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@SleeplessByte and @Cool-Katt,

I've made the necessary changes and implemented the getPrivateKey function along with the required tests. I ran format.mjs, lint.mjs, and test.mjs locally, and everything passed. However, the tests on GitHub are still failing. Could you help me identify the issue so I can fix it?

Copy link
Member

Choose a reason for hiding this comment

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

#2604

Not because of you. I'll figure that out.

Thanks for making the changes!

describe('private key is greater than 1 and less than p', () => {
const p = 23;
const g = 5;
const diffieHellman = new DiffieHellman(p, g);
Expand Down Expand Up @@ -87,4 +87,25 @@ describe('diffie-hellman', () => {

expect(secretA).toEqual(secretB);
});

xtest('private key is greater than 1 and less than p', () => {
let p = 23;
for (let i = 0; i < 10; i++) {
let privateKey = DiffieHellman.getPrivateKey(p);
expect(privateKey).toBeGreaterThan(1);
expect(privateKey).toBeLessThan(p);
}
});

xtest('private key is random', () => {
let p = 7919;
let uniqueKeys = new Set();
let testIterations = 1000;

for (let i = 0; i < testIterations; i++) {
uniqueKeys.add(DiffieHellman.getPrivateKey(p));
}

expect(uniqueKeys.size).toBeGreaterThan(testIterations - 100);
});
});
Loading