feat(BA-984): Add Action Tests for Image
#4048
Conversation
jopemachine
changed the title
ImageImage
jopemachine
force-pushed
the
refactor/image_action
branch
from
a875b72 to
5969762
Compare
jopemachine
force-pushed
the
feat/add_image_action_tests
branch
from
a9d334c to
4582ba7
Compare
jopemachine
force-pushed
the
feat/add_image_action_tests
branch
from
4582ba7 to
3a24196
Compare
jopemachine
force-pushed
the
feat/add_image_action_tests
branch
2 times, most recently
from
d1a4f81 to
16090e9
Compare
github-actions
bot
added
comp:manager
jopemachine
force-pushed
the
refactor/image_action
branch
from
2d0f6c7 to
d3dc584
Compare
jopemachine
force-pushed
the
feat/add_image_action_tests
branch
from
16090e9 to
e99275f
Compare
jopemachine
force-pushed
the
refactor/image_action
branch
5 times, most recently
from
b090bde to
387fa73
Compare
HyeockJinKim
force-pushed
the
refactor/image_action
branch
from
effb5fa to
818b1d0
Compare
jopemachine
force-pushed
the
feat/add_image_action_tests
branch
2 times, most recently
from
8314580 to
8cdd6ad
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @@ -44,6 +44,7 @@ | |||
| ) | |||
| if project: | |||
| stmt = stmt.where(ContainerRegistryRow.project == project) | |||
| # TODO: Raise exception if registry not found or two or more registries found | |||
Check notice
Code scanning / devskim
A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note
jopemachine
force-pushed
the
feat/add_image_action_tests
branch
from
bea6181 to
f8da2b9
Compare
| @@ -301,7 +301,7 @@ async def rescan_images( | |||
| # TODO: delete images removed from registry? | |||
|
|
|||
|
|
|||
| type Resources = dict[SlotName, dict[str, Decimal]] | |||
| type Resources = dict[SlotName, dict[str, Any]] | |||
There was a problem hiding this comment.
Is there a reason you changed this? @jopemachine
There was a problem hiding this comment.
There was a type error in the code below, so I temporarily changed it to Any, but I think it would be better to use int | str or define an appropriate type.
There was a problem hiding this comment.
It's very strange that it's updated because of the test code, is that correct?
There was a problem hiding this comment.
It seems that I added an incorrect type in the recent PR I submitted.
| if isinstance(col.type, sa.sql.sqltypes.DateTime): | ||
| for row in rows: | ||
| if col.name in row: | ||
| row[col.name] = isoparse(row[col.name]) |
There was a problem hiding this comment.
Wasn't there a datetime value acceptor before? Why did you add it?
There was a problem hiding this comment.
In the previous code for populate_fixture, the logic for handling DateTime types was missing, so an error occurs when generating a fixture that includes DateTime columns like ImageRow.created_at.
| ) | ||
| instance.id = data.id | ||
| return instance |
There was a problem hiding this comment.
Why don't you put the ID value at creation time?
There was a problem hiding this comment.
Since the ID is injected from the database, the ID value passed through the constructor wouldn't be passed to the database, so I intentionally excluded it from the constructor.
| @pytest.fixture | ||
| def processors(extra_fixtures, database_fixture, database_engine, registry_ctx): | ||
| agent_registry, _, _, _, _, _, _ = registry_ctx | ||
| image_service = ImageService(db=database_engine, agent_registry=agent_registry) | ||
| return ImageProcessors(image_service, []) |
There was a problem hiding this comment.
rename to image_processors
jopemachine
force-pushed
the
feat/add_image_action_tests
branch
from
1e471ff to
8aab5fd
Compare
jopemachine
force-pushed
the
feat/add_image_action_tests
branch
from
b6bb29b to
ded5ac5
Compare
Resolves #3964 (BA-984).
Based on #3997.
Test cases
Checklist: (if applicable)