[Tests] Use default config from config YAML in tests#392
[Tests] Use default config from config YAML in tests#392SumanthRH merged 13 commits intoNovaSky-AI:mainfrom
Conversation
Signed-off-by: SumanthRH <sumanthrh99@gmail.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a good initiative to refactor tests to use a default configuration from a YAML file, which enhances maintainability. The changes are generally well-aligned with this goal. However, I've identified a couple of critical issues that will prevent the tests from running correctly. The new get_default_config utility function misuses Hydra's API, which will lead to configuration loading failures. Additionally, a test helper function in tests/cpu/util.py has a bug where it returns None instead of a configuration object. I've provided code suggestions to fix these critical problems. I've also included a medium-severity comment to help complete a TODO for cleaning up redundant configuration overrides, further improving the code's clarity.
Signed-off-by: SumanthRH <sumanthrh99@gmail.com>
Signed-off-by: SumanthRH <sumanthrh99@gmail.com>
Signed-off-by: SumanthRH <sumanthrh99@gmail.com>
erictang000
left a comment
erictang000
left a comment
There was a problem hiding this comment.
lgtm, assuming you ran the tests and everything is passing, thanks!
|
Have confirmed that gpu tests also pass for |
# What does this PR do? Cleans up some tests to use the default config from the config YAML rather that using mocks or creating new OmegaConf object with all keys specified. This will make the tests more maintainable as we add more config parameters and will also help catch issues with the default config values. There are probably some more places where the replacement can be made, but this is a start. --------- Signed-off-by: SumanthRH <sumanthrh99@gmail.com>
2 participants
What does this PR do?
Cleans up some tests to use the default config from the config YAML rather that using mocks or creating new OmegaConf object with all keys specified. This will make the tests more maintainable as we add more config parameters and will also help catch issues with the default config values.
There are probably some more places where the replacement can be made, but this is a start.