deploy manifest bundle test #395
Conversation
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified FilesNo covered modified files...
|
| @@ -1804,3 +1804,39 @@ def test_make_html_bundle(): | |||
| "manifest.json", | |||
| ] | |||
| assert multi_file_index_dir_extras_ans == json.loads(tar.extractfile("manifest.json").read().decode("utf-8")) | |||
|
|
|||
|
|
|||
| pyshiny_manifest_dir = os.path.join(cur_dir, "./testdata/pyshiny_with_manifest") | |||
There was a problem hiding this comment.
It doesn't look like this variable is being used.
|
|
||
|
|
||
| pyshiny_manifest_dir = os.path.join(cur_dir, "./testdata/pyshiny_with_manifest") | ||
| pyshiny_manifest_file = os.path.join(cur_dir, "./testdata/pyshiny_with_manifest/manifest.json") |
There was a problem hiding this comment.
This variable doesn't need to be defined globally.
| pyshiny_manifest_file = os.path.join(cur_dir, "./testdata/pyshiny_with_manifest/manifest.json") | ||
|
|
||
|
|
||
| def test_make_manifest_bundle(): |
There was a problem hiding this comment.
This unit test is leaky.
It testing that the subsequent calls to other methods (bundle_add_file, read_manifest_file, etc) are working as expected, more so than testing the guts of make_manifest_bundle. The subsequent calls should be mocked, and this test should validate that the logic internal to make_manifest_bundle is working as expected.
For example, what happens if the 'files' key is missing in the manifest object?
I haven't looked, but there should be other unit tests that bundle_add_file, etc, are working correctly.
There was a problem hiding this comment.
The intent is to replace manifest and bundle related functions with the Manifest and Bundle classes, so the purpose of these specific tests are to document existing end to end behaviors. A refactored make_manifest_bundle most likely won't have the same logic, but its outputs must still be consistent with the existing behavior.
|
This test is a good addition, but limited to the happy path. Can you add some failure cases to the tests? For example, one of the main failure modes I'd expect to see with manifest-based deployments is missing files - ones that existed when the manifest was created but don't any more. Are there other scenarios that might (reasonably) arise in a real client environment that could break manifest deployments? |
Intent
Add deploy manifest bundle test as part of #376