-
Notifications
You must be signed in to change notification settings - Fork 12
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
315.fre list tool #329
base: main
Are you sure you want to change the base?
315.fre list tool #329
Conversation
- list experiments - list platforms
else: | ||
assert False | ||
|
||
@pytest.mark.xfail() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this test just expected to fail right now because of circumstances (e.g. CANNOT succeed in platform)? or is a specific failure supposed to occur because of e.g. raise
within one of the functions?
if former- OK. If latter, consult pytest
doc on expected exceptions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The former. This was just a little test thrown in there for if a bad yaml file was given (one that doesn't exist) - since -y
is the only click option passed. I do see now that this kind of test would be inthe test_fre_cli ... unit test file though. I'll try to thin of better tests for these tools
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a solid effort but i have questions before i click an approve button
- remove repetitive functions - use `pathlib` functions vs `os`
Describe your changes
Adds
fre list exps
andfre list platforms
tools, along with tests and some documentation.fre list exps
:fre list platforms
:Issue ticket number and link (if applicable)
#315
Checklist before requesting a review