-
Notifications
You must be signed in to change notification settings - Fork 9
add test for excel files #235
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
add test for excel files #235
Conversation
Added a step to capture logs from the auth service.
Downgrade Keycloak image version from 25.0.1 to 19.0.1 and set log level to DEBUG.
Updated healthcheck port for Keycloak service.
Updated CI workflow to improve Docker image caching and testing steps.
Modify log capture command to include timestamps and limit output.
Removed log capture step for auth service on failure.
krowvin
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.
Looks good, just question for my own understanding
| import pytest | ||
|
|
||
| import cwms | ||
| import cwms.catalog.blobs as blobs |
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.
Do we import these directly or would it be better to keep it as cwms?
I was thinking when I did this that having import cwms keeps it closer to what an enduser might use.
If we prefer to enforce a direct import from the module, then we might consider removing the imports from the root module?
i.e.
Line 4 in ac191f8
| from cwms.catalog.blobs import * |
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 see what you are saying about examples. but for tests it makes sense just to load what is being tested. when users use the package that should still use import cwms. but can also run it like above as well.
|



No description provided.