Skip to content

tests serializing of stage to lockfile #3988

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

Merged
merged 3 commits into from
Jun 9, 2020

Conversation

skshetry
Copy link
Collaborator

@skshetry skshetry commented Jun 9, 2020

  • ❗ I have followed the Contributing to DVC checklist.

  • πŸ“– If this PR requires documentation updates, I have created a separate PR (or issue, at least) in dvc.org and linked it here. If the CLI API is changed, I have updated tab completion scripts.

  • ❌ I will check DeepSource, CodeClimate, and other sanity checks below. (We consider them recommendatory and don't expect everything to be addressed. Please fix things that actually improve code or fix bugs.)

Thank you for the contribution - we'll try to review it as soon as possible. πŸ™

Part of #3693

@skshetry skshetry added the testing Related to the tests and the testing infrastructure label Jun 9, 2020
@skshetry skshetry self-assigned this Jun 9, 2020
`key_vals` - which is list of params with values, used in a lockfile
which is in the shape of:

def _serialize_params_values(params: List[ParamsDependency]):
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Splitted serialization of params to serializing keys and values. Was getting too complex to understand.

@skshetry skshetry force-pushed the tests-serialize-lockfile branch from 17b0fb9 to 86d7c13 Compare June 9, 2020 11:31
Comment on lines +17 to +18
force_grid_wrap=0
use_parentheses=True
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why I added this is because, isort adds a \ on hanging indent for imports:

from dvc.stage.serialize import \
    to_single_stage_lockfile as _to_single_stage_lockfile

Whereas black formats it into this:
vs:

from dvc.stage.serialize import (
    to_single_stage_lockfile as _to_single_stage_lockfile
)

class TestPathConversion(TestCase):
def test(self):
stage = Stage(None, "path")
def test_path_conversion(dvc):
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Converted to pytest format.

@skshetry skshetry marked this pull request as ready for review June 9, 2020 11:36
@skshetry skshetry requested review from efiop, pared and pmrowla June 9, 2020 11:36
@skshetry skshetry changed the title [WIP] tests serializing of stage to lockfile tests serializing of stage to lockfile Jun 9, 2020
Comment on lines +34 to +39
assert to_single_stage_lockfile(stage) == OrderedDict(
[
("cmd", "command"),
("deps", [OrderedDict([("path", "input"), ("md5", "md-five")])]),
]
)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the order is very important for the lockfile, most of the tests use ugly OrderedDict.

@efiop efiop merged commit afc183f into iterative:master Jun 9, 2020
@skshetry skshetry deleted the tests-serialize-lockfile branch June 9, 2020 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing Related to the tests and the testing infrastructure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants