Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Add facilities for unit and functional tests#165

Merged
mergify[bot] merged 1 commit intoinstructlab:maininstructlab/eval:mainfrom
danmcp:unittestsdanmcp/eval:unittestsCopy head branch name to clipboard
Nov 5, 2024
Merged

Add facilities for unit and functional tests#165
mergify[bot] merged 1 commit intoinstructlab:maininstructlab/eval:mainfrom
danmcp:unittestsdanmcp/eval:unittestsCopy head branch name to clipboard

Conversation

@danmcp
Copy link
Contributor

@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
Closed
@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

.github/workflows/test.yml Show resolved Hide resolved
@mergify mergify bot added the one-approval label Nov 1, 2024
@danmcp
Copy link
Contributor 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
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 <dmcphers@redhat.com>
@mergify mergify bot removed the ci-failure label Nov 1, 2024
@mergify mergify bot merged commit 4c76340 into instructlab:main Nov 5, 2024
@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

2 more reviewers

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

@alimaredia alimaredia alimaredia approved these changes

Reviewers whose approvals may not affect merge requirements

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

Comments

Close sidebar
Morty Proxy This is a proxified and sanitized view of the page, visit original site.