Skip to content

Add facilities for unit and functional tests #165

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

Jump to bottom
Merged
merged 1 commit into from
Nov 5, 2024
Merged

Add facilities for unit and functional tests #165

merged 1 commit into from
Nov 5, 2024

Conversation

@danmcp
Copy link
Member

@danmcp danmcp commented Oct 31, 2024 •
edited
Loading

There was already 1 unit test in place. The existing functional tests are being moved to scripts for consistency with other projects.

One new functional test has been added to test the mt_bench_branch generator as a starting example

I've included basic tests for mt_bench and mmlu. Which already found two issues:

  • mmlu_branch was using self.tasks instead of the number of results calculating the wrong average
  • Not specifying max_workers for mt_bench resulted in an error when it should have defaulted

@mergify mergify bot added CI/CD Affects CI/CD configuration documentation Improvements or additions to documentation testing Relates to testing ci-failure labels Oct 31, 2024
@mergify mergify bot added ci-failure and removed ci-failure labels Oct 31, 2024
@mergify mergify bot added ci-failure and removed ci-failure labels Oct 31, 2024
@mergify mergify bot removed the ci-failure label Oct 31, 2024
This was referenced Oct 31, 2024
Merged
Open
@mergify mergify bot added the ci-failure label Oct 31, 2024
@mergify mergify bot removed the ci-failure label Oct 31, 2024
@mergify
Copy link
Contributor

mergify bot commented Oct 31, 2024

This pull request has merge conflicts that must be resolved before it can be
merged. @danmcp please rebase it. https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify bot added the needs-rebase label Oct 31, 2024
@mergify mergify bot removed the needs-rebase label Oct 31, 2024
@danmcp danmcp force-pushed the unittests branch 2 times, most recently from 32b8217 to a5dc505 Compare October 31, 2024 22:57
nathan-weinberg
nathan-weinberg approved these changes Nov 1, 2024
View reviewed changes
Copy link
Member

@nathan-weinberg nathan-weinberg left a comment

Choose a reason for hiding this comment

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

Looked over all these changes and honestly everything LGTM - thank you for this @danmcp!

Two nits - one is in the comment, the other would be it'd be nice to add a badge for this new workflow similar to what is done for the CLI here: https://github.com/instructlab/instructlab/blob/main/README.md?plain=1#L4

@mergify mergify bot added the one-approval label Nov 1, 2024
@danmcp
Copy link
Member Author

danmcp commented Nov 1, 2024

the other would be it'd be nice to add a badge for this new workflow similar to what is done for the CLI here: https://github.com/instructlab/instructlab/blob/main/README.md?plain=1#L4

Done!

@mergify mergify bot added the ci-failure label Nov 1, 2024
@danmcp
Add facilities for unit and functional tests
5cff4ab 
There was one unit test already existing and this commit adds a couple of starter unit tests for mt_bench and mmlu.

Signed-off-by: Dan McPherson <[email protected]>
@mergify mergify bot removed the ci-failure label Nov 1, 2024
@mergify mergify bot merged commit 4c76340 into instructlab:main Nov 5, 2024
20 checks passed
@mergify mergify bot removed the one-approval label Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nathan-weinberg nathan-weinberg nathan-weinberg approved these changes

@alimaredia alimaredia alimaredia approved these changes

Assignees

No one assigned

Labels

CI/CD Affects CI/CD configuration documentation Improvements or additions to documentation testing Relates to testing

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@danmcp @alimaredia @nathan-weinberg