Stop running some nativeapp tests on v1 projects and ws command #1699
Conversation
sfc-gh-fcampbell
force-pushed
the
frank-remove-v1-test-projects
branch
2 times, most recently
from
0418b21 to
2a93612
Compare
|
How comprehensive are those v1 -> v2 tests? I understand that running more integ tests is slower overall, and if we aren't getting anything for them, we should remove them. However, we need to make sure that we don't let v1 be impacted by new development, as the vast majority of our users are still on v1. What's our plan to ensure that we don't break v1 in the future? |
Pull request was converted to draft
sfc-gh-fcampbell
force-pushed
the
frank-remove-v1-test-projects
branch
from
34b7945 to
1b97354
Compare
sfc-gh-fcampbell
changed the title
| @pytest.fixture(params=["napp_init_v1", "napp_init_v2"]) | ||
| def _test_project(request): | ||
| return request.param | ||
|
|
||
|
|
||
| @pytest.fixture(params=["app version create", "ws version create --entity-id=pkg"]) | ||
| def _create_command(request): | ||
| return request.param | ||
|
|
||
|
|
||
| @pytest.fixture(params=["app version list", "ws version list --entity-id=pkg"]) | ||
| def _list_command(request): | ||
| return request.param | ||
|
|
||
|
|
||
| @pytest.fixture(params=["app version drop", "ws version drop --entity-id=pkg"]) | ||
| def _drop_command(request): | ||
| return request.param | ||
|
|
||
|
|
||
| @pytest.fixture() | ||
| def command_parametrization( | ||
| _create_command, _list_command, _drop_command, _test_project | ||
| ): | ||
| if "v1" in _test_project and ( | ||
| "ws" in " ".join([_create_command, _list_command, _drop_command]) | ||
| ): | ||
| pytest.skip("ws commands are not supported on v1 projects") | ||
| return _create_command, _list_command, _drop_command, _test_project |
There was a problem hiding this comment.
sfc-gh-fcampbell
force-pushed
the
frank-remove-v1-test-projects
branch
from
1b97354 to
9534700
Compare
Reworked this PR and updated the description to describe what we're doing now. |
| @pytest.mark.parametrize( | ||
| "create_command,list_command,drop_command,test_project", | ||
| [["app version create", "app version list", "app version drop", "napp_init_v2"]], | ||
| ) |
There was a problem hiding this comment.
I assume this is for consistency, since there doesn't seem to be a need to use a parametrized test here?
There was a problem hiding this comment.
yeah, I wanted to keep the parametrization for two reasons, 1. it makes this PR's diff smaller, and 2. it puts the relevant info in the test signature rather than in the body, keeping the test IDs consistent for stuff like snapshot names, debugging, etc
Pre-review checklist
Changes description
Since we now convert v1 definitions to v2 in-memory when running
snow appcommand, we don't need to run all integration tests for v1 example projects anymore. Also, since the status of thesnow wscommand is uncertain (snow appandsnow wsnow call the same code under the hood), we're removing thesnow wsparametrization from most tests. All in all, this reduces the Native App integration test suite from 300 unique tests to 128, which greatly speeds it up (from about 35 minutes to 19 minutes).For safety, we're keeping full parametrization on a simple test for each feature, to make sure that the v1 to v2 conversion and
snow wscommand don't regress.