feat: chalk insert lambda zip mode #498
Conversation
| field lambda_mode { | ||
| type: bool | ||
| default: false | ||
| shortdoc: "lambda mode" | ||
| doc: """ | ||
| When present, inject the executed chalk binary itself in the zip | ||
| archive being processed upon insertion. | ||
| """ | ||
| } |
There was a problem hiding this comment.
this is inherently not anything lambda specific. lets change terminology to be more generic? maybe insert_chalk_binary?
| var isZip = false | ||
| if item.collectedData != nil and "ARTIFACT_TYPE" in item.collectedData: | ||
| try: | ||
| isZip = item.collectedData["ARTIFACT_TYPE"] == artTypeZip | ||
| except: | ||
| isZip = false | ||
|
|
||
| if isZip: | ||
| info(item.name & ": inserting binary into zip") |
There was a problem hiding this comment.
this does not belong here. it should be directly in the zip codec as you mentioned in the PR description.
chalk/src/plugins/codecZip.nim
Lines 168 to 196 in d332e41
it already unpacks the zip file in there. we just need to add the chalk binary in there in-place
| def test_lambda_insert_empty_zip( | ||
| tmp_data_dir: Path, | ||
| chalk: Chalk, | ||
| ): | ||
| """Test `chalk insert --lambda` on an empty zip file. | ||
|
|
||
| Empty zip files should be handled gracefully - the command should | ||
| complete with exit code 0 but no chalk marks should be created. | ||
| """ | ||
|
|
||
| # Run chalk insert with the --lambda | ||
| insert = chalk.insert( | ||
| artifact=tmp_data_dir, |
There was a problem hiding this comment.
😕 description does not matchwhat the test case does. it runs on empty empty folder which is not related to empty zip files
tests/functional/test_lambda.py
Outdated
| assert ( | ||
| "_CHALKS" not in insert.report | ||
| ), "No chalks should be created for empty zip file" | ||
| assert ( | ||
| "_OP_CHALK_COUNT" in insert.report and insert.report["_OP_CHALK_COUNT"] == 0 | ||
| ), "Chalk count should be 0" |
There was a problem hiding this comment.
these are already handled by these options:
chalk/tests/functional/chalk/runner.py
Lines 433 to 434 in d332e41
they will assert appropriate values in the report are present
tests/functional/test_lambda.py
Outdated
| verify_chalk_mark_added(insert, test_file) | ||
|
|
||
| # Now extract and verify we can read the chalk mark | ||
| extract = chalk.extract(artifact=tmp_data_dir, ignore_errors=True) |
There was a problem hiding this comment.
why ignore errors? this should only be used for commands where we legitimately expect errors such as trying to load bad chalk config in tests
| # chalk_binary_path = tmp_data_dir / "chalk" | ||
| # Verify ZIP artifact type is detected | ||
| for mark in extract.marks: | ||
| assert mark.has(ARTIFACT_TYPE="ZIP"), "ZIP artifact type not detected" |
There was a problem hiding this comment.
this is already tested by extract util
chalk/tests/functional/chalk/runner.py
Lines 494 to 500 in d332e41
tests/functional/test_lambda.py
Outdated
| ], | ||
| indirect=True, | ||
| ) | ||
| def test_lambda_with_other_zip_formats( |
There was a problem hiding this comment.
not following intent here?
if we want chalk to be able to support multiple zip formats, we should just have a single test case with parametrize inputs which will run the test case on all the variants. from the test case perspective it should not matter how zip was created
b976cf9 to
5781aa5
Compare
CHANGELOG.mdif necessaryIssue
https://app.shortcut.com/crashoverride-1/epic/4279?group_by=none&ct_workflow=all&cf_workflow=500000306
Description
Initial approach for injecting
chalkbinary inside of a lambda zip archive. After a round of feedback I think it'll make sense to refactor some of the zip archive operations tocodecZip.nim.to do
codecZip.nimTesting
make tests args="test_lambda.py"