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

Added sidarthe scenario and relax constraints for test runner #30

Merged
merged 2 commits into from
Aug 30, 2023

Conversation

brandomr
Copy link
Contributor

@brandomr brandomr commented Aug 30, 2023

What's New

This PR adds some basic tests for SIDARTHE. It also relaxes constraints around the resource requirements to run tests when MOCK_TA1 is False.

Here's the rationale--we should make it as easy as possible to add test scenarios. Requiring people to come up with everything a priori is perhaps a tough pill to swallow. For example--we should be able to run a basic scenario with just the code and paper. Unfortunately because of our interaction with SKEMA for the extractions we need the Cosmos JSON 👎

Another thing to note is that I changed the utility that gathers the scenarios so that if MOCK_TA1 is True then we only run the basic scenario since we won't assume to have all the correct resources needed to truly mock TA1.

I think this is likely a key discussion point for tomorrow. TA1 folks may not have a problem pre-generating all the requisite resources but I find it pretty painful to do. Obviously these are required for quality checks, but not for the core integration test necessarily.

Copy link
Collaborator

@fivegrant fivegrant left a comment

Choose a reason for hiding this comment

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

It's not great that we need to split up what's tested between mocked and unmocked BUT I think you're right that we need to relax the constraints. We can't get around it unless TA1 buys into generating the resources.

I only request one change. Feel free to accept or reject it and I'll approve

tests/utils.py Outdated Show resolved Hide resolved
Co-authored-by: Five Grant <5@fivegrant.com>
@brandomr brandomr merged commit 6ae2382 into main Aug 30, 2023
1 check passed
@brandomr brandomr deleted the br/generate-scenarios branch August 30, 2023 13:40
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.

2 participants