iterative / dvc

🦉 ML Experiments and Data Management with Git
https://dvc.org
Apache License 2.0
13.67k stars 1.17k forks source link

plots: Expand flexible plots tests #7956

Open daavoo opened 2 years ago

daavoo commented 2 years ago

Yes, its true. I will fix that in follow up PR, where I intend to parameterized this test to handle one use case per test. That way we won't have to create this huge repo_with_plots. This specific use case (list of columns) is tested in tests/unit/render/test_vega_converter.py::test_converter - its a unit test but should be enough for now.

_Originally posted by @pared in https://github.com/iterative/dvc/pull/7477#discussion_r910856251_

pared commented 2 years ago

Aim of this issue is to parametrize tests in tests/integration/plots/test_plots.py, so that we have single use case per parameter set, and not testing few use cases at once, as we do now. That will allow us to test every expected plots set up with a test that is much easier to comprehend.

pared commented 2 years ago

Need to test #8004 too

pared commented 2 years ago

Include test for #8159

dberenbaum commented 1 year ago

@daavoo What's still needed here?

daavoo commented 1 year ago

@daavoo What's still needed here?

No idea off the top of my mind. I would need to take a look. At this point, I think most behaviors we care about are somehow tested in here or dvc-render or studio. But revisiting the tests would not hurt

dberenbaum commented 1 year ago

Okay, feel free to close this one or take another look.