-
Notifications
You must be signed in to change notification settings - Fork 322
test: microgen - adds unit tests for utils.py
#2307
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
Conversation
utils.pyutils.py
.github/workflows/unittest.yml
Outdated
| strategy: | ||
| matrix: | ||
| python: ['3.9', '3.10', "3.11", "3.12", "3.13"] | ||
| python: ["3.13"] |
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.
Aren't you removing the unit test matrix for all the normal testing by this change?
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.
@shollyman
Yes. This code is focused on the microgenerator and getting ready for alpha release.
It is less worried about the code that the GAPIC generator produces and all the GAPIC generated unit tests.
In order to simplify figuring out how this workflow would work along with the regular GAPIC unittest workflow, I opted to reduce it to a single runtime to speed up iteration and experimentation.
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.
I added a comment explaining that this is an experiment for the alpha release.
We are simply attempting to better understand the testing process for two disparate parts of the codebase in the event that we choose to keep the microgenerator as some part of the main package (I think we will want to move it into a package of it's own, but that is a story for another day).
.github/workflows/unittest.yml
Outdated
| COVERAGE_FILE: .coverage-${{ matrix.python }} | ||
| BUILD_TYPE: presubmit | ||
| TEST_TYPE: unit | ||
| TEST_TYPE: unit xxx |
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.
I'm not clear why this is added. Is it used to parameterize the output in some way?
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.
I am trying to identify how the task names and TEST_TYPEs reflect in the outputs displayed in the GH user interface, so I dropped in some characters to see what shows up and what does not.
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.
i added some comments and renamed some things.
| PY_VERSION: ${{ matrix.python }} | ||
| run: | | ||
| nox -s unit-${{ matrix.python }} | ||
| - name: Run microgenerator unit tests |
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.
If you think microgenerator testing is sufficiently distinct from the normal testing, another strategy here would be to define it all in an addition, custom workflow rather than in unitttest.yml
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.
You are correct, that is another strategy.
It seemed reasonable at first glance to put all unit tests into a unittests workflow to get us in a position to be able to test a unit test setup for the microgenerator with the minimum amount of fuss.
If I can't get this work, I may try that approach.
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.
shollyman
left a comment
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.
Missed this was landing in autogen rather than main


Adds unit tests to exercise the functions in
utils.py._load_resource() # used by both _load_template() and _load_config()_load_template()_load_config()walk_codebase()write_code_to_file()