UI-6384 - Test the CLI #4
Conversation
There was a problem hiding this comment.
The test looks good, but I am wondering about the best approach for this. I think it is out of the scope of this PR, as similar tests already existed in this repo.
My concern is huge snapshots, and in particular with parts of JSON objects being stringified and inlined (the content of CS entries representing dashboards). My concern is that it is so hard to read that I believe developers will never care about snapshot changes. Maybe the most thorough ones will just pay attention to the fact that they changed in case they did not expect it, but not to the actual change. And even this, I believe that the majority of developers will not even do it to be honest.
That being said, it is good to have some tests checking the input/output of the CLI and I don't think this is debatable.
Maybe the best option would be to serialize snapshots in a better way, in particular at least to avoid inlining the dashboard states.
What do you think?
|
I agree, I'm not a fan of huge snapshots either. We could decide that the more atomic unit tests should be enough, and that in the CLI test we should just test that there are no errors being thrown. But if we want to keep testing the output, I don't really see a way around the huge snapshot here. |
|
Cannot auto-update because of conflicts. |
|
I am in favor of keeping the big snapshots for now and moving this discussion to a later time. Other than that the PR looks good. Can you rebase it? Also, I noticed that tests are broken on master. We should add a CI to this repo. Could you open a ticket to track it? |
|
antoinetissier
force-pushed
the
personal/a.tissier/UI-6384-test-cli
branch
from
9e04b49 to
8bd541f
Compare
|
Cannot auto-update because of conflicts. |
🧪