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.

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

Additional tests #1369

Open
wants to merge 17 commits into
base: dev
Choose a base branch
from
Open

Additional tests #1369

wants to merge 17 commits into from

Conversation

7h3kk1d
Copy link
Contributor

@7h3kk1d 7h3kk1d commented Aug 10, 2024

No description provided.

@@ -3,7 +3,10 @@ open Junit_alcotest;
let (suite, _) =
run_and_report(
~and_exit=false,
"Dynamics",
[("Elaboration", Test_Elaboration.elaboration_tests)],
"HazelTests",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's still very unclear to me at what level this report is supposed to be built at. A lot of sample repos online just have the full suite or tests. The alcotest documentation lists a module but no clear way to have a bunch of modules in a report.

Comment on lines 8 to 9
("Elaboration", Test_Elaboration.elaboration_tests),
Test_ListUtil.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.

I prefer naming the tests in the test module but 🤷

Comment on lines 15 to 28
/**
Groups elements of a list by a specified key.

{b Note: The groups are not guaranteed to preserve the order of elements from the original list. }

@param key
The key function used to determine the grouping key.

@param xs
The list of elements to be grouped.

@return
A list of tuples where each tuple contains the grouping key and a list of elements that belong to that group.
*/
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Generated odoc for ListUtil

Gearing up for a future odoc pr. Mostly added this so the ordering is noted.

`Quick,
() => {
let xs = [1, 2, 3, 2];
check(list(int), "Unique list", [1, 3, 2], ListUtil.dedup(xs)); // TODO: Interesting the order here is messed up because of fold_right
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 can add odoc comments for these if it's how we want to handle it

@7h3kk1d 7h3kk1d marked this pull request as ready for review August 19, 2024 15:44
src/util/ListUtil.re Outdated Show resolved Hide resolved
Copy link
Contributor

@Negabinary Negabinary left a comment

Choose a reason for hiding this comment

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

I'm glad ListUtil works

src/util/ListUtil.re Outdated Show resolved Hide resolved
Copy link
Member

@disconcision disconcision left a comment

Choose a reason for hiding this comment

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

lgtm

@7h3kk1d 7h3kk1d requested a review from cyrus- August 27, 2024 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants