fix: chart sorter #1400
fix: chart sorter #1400
Conversation
| @@ -813,9 +810,6 @@ describe('ChartDataRequestBuild Test', () => { | |||
| builder.addExtraSorters(extraSorters2); | |||
|
|
|||
| expect(requestParams.orders).toEqual([ | |||
| { column: 'first-name', operator: 'ASC', aggOperator: undefined }, | |||
| { column: 'last-name', operator: 'DESC', aggOperator: undefined }, | |||
| { column: 'address', operator: 'DESC', aggOperator: undefined }, | |||
There was a problem hiding this comment.
should get orders with unique extra sorters 这个测试不就是在做这个事情吗，这里把之前的旧逻辑给修改了一下，满足了现有的逻辑。这是我的理解，不太明白你的意思。还望展开讲讲。
不怎么写单元测试，见谅🌚
There was a problem hiding this comment.
单元测试的目的是尽可能多的覆盖逻辑分支，例如if...else if...else if...这种不但要测试if中的逻辑内容，也要尽可能多的覆盖else if分支中的逻辑，从可读性角度来讲，一般一个逻辑分支一个测试名称会比较合适，如果需要再多可以进行测试分组如describe。如果能从Test Driven Design的角度来编写函数就更能体会到如果对逻辑分支编写测试，而不是我们现在这种完成了功能之后倒推测试（从程序设计者的角度来讲，先功能后测试和先测试后功能，不仅仅是花费时间上的差异，更重要的其实往往是对问题的拆解的能力）。关于TDD的内容如果不熟悉，可以参考老马的TDD文章
关于当前这个测试，确实是我表达过于省略。原来sort的逻辑是当有extra和origin sorters时，二者叠加，同时保持原有的extra不被清空（这一点没有单独编写成一个测试也违背了“一个逻辑分支一个测试的最佳实践”）。而，更新后的逻辑是，如果存在extra则使用extra，否则返回origin。
从这里可以看出如果要覆盖以上逻辑，需要三个独立的测试：
- 当只有origin时，返回origin
- 当有extra和origin时，返回extra
- 当builder原来有extra，后来即使origin发生了变化，依然返回原有的extra
以上只是个人的一点想法，如果对于测试有想法和讨论欢迎随时交流。测试并不强求，而且Datart目前测试覆盖率整体也在10%左右，我们尽量将比较容易测试的、易变动的工具函数进行高覆盖率测试。
|
感谢参与贡献！ |
基本保留现有实现，只改一条：在预览页面用户运行时操作某一列排序时、预设的排序条件失效，仅该列排序条件生效；当运行时排序取消时，预设条件生效。