WIP: Add lightweight unit tests#20
Open
yonatank93 wants to merge 19 commits into
Open
Conversation
…ult values In this version, the keys and values in the configuration file are treated as keyword arguments to instantiate classes. This makes the behavior of running with configuration file/factory the same as if we instantiate the classes manually. For the environment class, the dataset argument is replaced with dataset_path, so that one can pass in file without needing to load the file first. Special treatment is for the configurations inside AutoREC/default_configs, where the dataset_path there is treated as path relative to the configuration file instead of to the current directory. This is because those default configurations are used for example/testing.
…nt treatment of dataset_path
…efault_configs I also created a new default configs for the docker run demo. Instead on adding more function in factory to also handle this case, I think it is just easier to just have an additional config file. It is not big anyway.
… run with minimum argument
Update the error message Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…oREC into 16_fix_config_file_behavior
Copilot suggestion for potential bug related to setting the save_dir in the agent configuration yaml file. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Update printed message. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Since there are already some data included, like pretrained model, dataset, etc., I think doing this reorganization makes it more consistent structure wise with the others.
The unit tests check: 1. EIS_Data_Prep module and the data stuctures used, like buffers 2. DDQN_ECM agent module 3. EIS_ECM_Env module 4. factory builder functions 5. EIS parser functions 6. Utility function 7. Command line interface builder 8. Runtime utility functions
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a new lightweight unit test suite across core AutoREC modules and refactors configuration/factory plumbing to better support example YAML configs (including Docker paths). It also fixes a long-standing attribute typo (EIS_INPUT_SZE → EIS_INPUT_SIZE) and updates documentation/examples accordingly.
Changes:
- Add pytest unit tests for utilities, CLI parsing, factory config resolution, runtime setup, data prep, environment helpers, agent wiring, and optimized data structures.
- Refactor
factory.py/cli.pyto use a newconfig_readerthat applies special dataset path handling for example configs (repo + Docker layouts), and remove default-config merging behavior. - Update README/examples/Docker packaging to reference
configs_yaml/examples/instead of prior default config locations; fix env/agent attribute typo linkage.
Reviewed changes
Copilot reviewed 25 out of 28 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
tests/conftest.py |
Adds global pytest env setup to redirect caches/config writes for heavy deps. |
tests/test_utils.py |
Adds unit tests for small utility functions (CSV/PKL saving, encoding, validity checks). |
tests/test_runtime.py |
Adds tests for runtime directory/env-var setup. |
tests/test_parser.py |
Adds tests for parser helpers and simplification logic. |
tests/test_factory.py |
Adds tests for factory path-resolution behavior and dataset loading errors. |
tests/test_environment.py |
Adds environment setup/reset/reward/cache-behavior tests with stubbing. |
tests/test_data_structures.py |
Adds tests for CircularBuffer and cache eviction/stats behavior. |
tests/test_data_preparation.py |
Adds tests for EISDataPrep normalization/interpolation/load/save/process plumbing. |
tests/test_cli.py |
Adds tests for CLI parsing and YAML override behavior. |
tests/test_agent.py |
Adds tests for DDQN agent defaults/env switching with model/optimizer stubs. |
src/autorec/factory.py |
Introduces config_reader and changes factory behavior around example configs + defaults removal. |
src/autorec/cli.py |
Switches to config_reader and hardens agent override handling. |
src/autorec/environment.py |
Fixes EIS_INPUT_SIZE attribute typo. |
src/autorec/agent.py |
Updates model setup to use EIS_INPUT_SIZE. |
README.md |
Updates paths and docs for new config location and dataset_path semantics. |
pyproject.toml |
Removes package-data entry for now-removed default config YAMLs. |
example/main_factory.py |
Updates example runner to accept CLI args and use config_reader. |
example/hyperparameter_tuning/main_factory.py |
Updates default config path for tuning example. |
Dockerfile |
Copies configs_yaml/ into the image instead of old default config dir. |
configs_yaml/examples/environment_config.yaml |
Updates dataset_path to be relative to the example config directory. |
configs_yaml/examples/environment_agent_config.yaml |
Updates example dataset_path resolution. |
configs_yaml/examples/demo_environment_agent_config.yaml |
Updates demo config dataset_path resolution. |
configs_yaml/examples/demo_docker_environment_agent_config.yaml |
Updates Docker demo paths and trims runtime-heavy defaults. |
configs_yaml/examples/agent_config.yaml |
Adds example agent config YAML. |
default_configs/environment_config.yaml |
Removes legacy config location/file. |
src/autorec/default_configs/agent_config.yaml |
Removes legacy packaged default agent config. |
.gitattributes |
Normalizes formatting (whitespace-only change). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… for future proofing Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Comment on lines
131
to
135
| agent_config = config["agent"] | ||
| # Insert default values for missing parameters | ||
| agent_config = _insert_defaults(agent_config, default_agent_config) | ||
| if agent_config is None: | ||
| print("Warning: The 'agent' section is empty. Using DDQN_ECM constructor defaults.") | ||
| agent_config = {} | ||
| # Insert the environment(s) into the agent configuration |
Comment on lines
+167
to
+170
| def config_reader(file_path: Union[str, Path]) -> Dict: | ||
| """Read a config file and apply example config path conventions.""" | ||
| config = _yaml_reader(file_path) | ||
| config_path = Path(file_path).resolve() |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Major change is addition of lightweight unit tests
Related issue
#18
Type of change
Please check all that apply:
Motivation and context
Unit tests are needed to ensure that any changes we make in the future won't break the package.
Additional dependencies introduced, if any
Not applied. Pytest is already included as a dependency.
TODO, if any
Checklist
Before this pull request is merged, please check the following:
Additional comments
These unit tests might not be sufficient for a complete check, but at least if the branch passes all unit tests, it is more likely that it doesn't break anything.
However, developers are still encouraged to double check their changes.
Furthermore, if the branch breaks but all unit tests pass, then the unit tests needs to be expanded to capture those breaking points.