Skip to content

Conversation

@iamemilio
Copy link
Contributor

@iamemilio iamemilio commented May 31, 2025

A very basic Open AI "Like" completions API server for mocking.

Something to note: OpenAI Like completion responses are based on the legacy completions APIs, and are not compatible with the latest OpenAI completions APIs

@mergify mergify bot added documentation Improvements or additions to documentation ci-failure labels May 31, 2025
@mergify mergify bot added ci-failure and removed ci-failure labels May 31, 2025
@iamemilio iamemilio force-pushed the inference-mock branch 3 times, most recently from 9f86c66 to 36635e8 Compare June 1, 2025 22:31
@mergify mergify bot added ci-failure and removed ci-failure labels Jun 1, 2025
@mergify mergify bot removed the ci-failure label Jun 1, 2025
@iamemilio
Copy link
Contributor Author

reference passing test job: instructlab/examples#22

@iamemilio iamemilio marked this pull request as ready for review June 2, 2025 14:21
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice. Can we add unit tests for these new scripts in case others want to contribute to them in the future?

@mergify mergify bot added the ci-failure label Jun 2, 2025
Signed-off-by: Emilio Garcia <[email protected]>
@mergify mergify bot added the CI/CD Affects CI/CD configuration label Jun 2, 2025
@iamemilio iamemilio force-pushed the inference-mock branch 4 times, most recently from e19174d to ba12a30 Compare June 2, 2025 19:26
@mergify mergify bot added ci-failure and removed ci-failure labels Jun 2, 2025
Signed-off-by: Emilio Garcia <[email protected]>
@mergify mergify bot added ci-failure and removed ci-failure labels Jun 2, 2025
Signed-off-by: Emilio Garcia <[email protected]>
@mergify mergify bot removed the ci-failure label Jun 2, 2025
Copy link
Contributor

@ktdreyer ktdreyer left a comment

Choose a reason for hiding this comment

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

In the future I think we should use pytest's newer style tests instead of unittest.

You can switch the old-style unittest asserts to use bare assert statements. For example like self.assertIsInstance can be assert isinstance(...) . pytest will give you rich assertion logs when things fail.

The current implementation is ok and I'll approve here.

@iamemilio
Copy link
Contributor Author

iamemilio commented Jun 2, 2025

In the future I think we should use pytest's newer style tests instead of unittest.

You can switch the old-style unittest asserts to use bare assert statements. For example like self.assertIsInstance can be assert isinstance(...) . pytest will give you rich assertion logs when things fail.

The current implementation is ok and I'll approve here.

can you link me a guide on it? I am not super familiar with whats current in testing in python. I spent the last 8 years doing Go exclusively, so this is kinda new to me

@mergify mergify bot removed the ci-failure label Jun 2, 2025
@ktdreyer
Copy link
Contributor

ktdreyer commented Jun 2, 2025

https://docs.pytest.org/en/stable/getting-started.html is a great introduction.

Here are the changes I recommend to switch this code to pytest:

  1. Update the GitHub action run statement to include pytest:

    pip install pytest -r requirements.txt
    
  2. Rename the test directory to tests so that pytest discovers it automatically EDIT: whoops, this is not needed - I learned something new today

  3. Rename test.py to test_inference_mock.py so that pytest will invoke it. It invokes any files that match the test_*.py pattern.

  4. At this point, when you invoke pytest in your inference-mock directory, you will see it discovers and runs all the unit tests automatically.

  5. Instead of running python -m unittest test/test.py in the GitHub action run statement, run pytest with no arguments.

Now that GHA is running the tests with pytest, you can clean up the unittest references.

  1. Change your test classes like TestAlways to not inherit unittest.TestCase. Just remove the () so the class definition is like:

    class TestAlways:
        # match on any prompt 
        def test_always(self):
            ...
  2. Instead of self.assertEqual(actual_response, expect_response), you can use assert actual_response == expect_response

  3. self.assertIsNone(actual_response) becomes assert actual_response is None

  4. self.assertIsInstance(contains, Contains) becomes assert isinstance(contains, Contains)

  5. To replace assertRaises, use the pytest.raises context manager. import pytest at the top of the file, then:

with pytest.raises(ValueError):
    to_match(invalid_pattern)
  1. Remove import unittest at the top

  2. Remove this, because you will invoke the tests with pytest now:

if __name__ == "__main__":
    unittest.main()

@iamemilio
Copy link
Contributor Author

iamemilio commented Jun 2, 2025

Thanks! I will make a follow up PR and fix this up! 🚀

Copy link
Contributor

@courtneypacheco courtneypacheco left a comment

Choose a reason for hiding this comment

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

Nice work! Looks great! Sorry I didn't get to review this right away.

@mergify mergify bot merged commit 8644c87 into instructlab:main Jun 6, 2025
14 checks passed
@mergify mergify bot removed the one-approval label Jun 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants