shardingsphere icon indicating copy to clipboard operation
shardingsphere copied to clipboard

Refactor DistSQLExecutor's test cases

Open terrymanu opened this issue 1 year ago • 13 comments

At present, we use the raw executor to perform assertion on test cases. This approach is not accurate, as it requires manual injection of required dependencies such as database and rule. I hope to use DistSQLQueryExecuteEngine and DistSQLUpdateExecuteEngine, which can create the executor through SPI, and automatically inject dependencies.

The sample please refer to ShowAuthorityRuleExecutorTest.

At present, a large number of DistSQLExecutor test cases need to be updated, and we sincerely seek the help of the community.

The test cases are:

  • [x] ShowGlobalClockRuleExecutorTest
  • [x] AlterGlobalClockRuleExecutorTest
  • [x] ShowSingleTableExecutorTest
  • [x] ShowDefaultSingleTableStorageUnitExecutorTest
  • [ ] LoadSingleTableExecutorTest
  • [ ] SetDefaultSingleTableStorageUnitExecutorTest
  • [x] ShowSQLFederationRuleExecutorTest
  • [x] AlterSQLFederationRuleExecutorTest
  • [x] ShowSQLParserRuleExecutorTest
  • [x] AlterSQLParserRuleExecutorTest
  • [x] ShowSQLTranslatorRuleExecutorTest
  • [x] AlterSQLTranslatorRuleExecutorTest
  • [x] ShowTrafficRuleExecutorTest
  • [x] AlterTrafficRuleExecutorTest
  • [x] ShowTransactionRuleExecutorTest
  • [ ] AlterTransactionRuleExecutorTest
  • [x] ShowBroadcastTableRuleExecutorTest
  • [ ] CreateBroadcastTableRuleExecutorTest
  • [ ] DropBroadcastTableRuleExecutorTest
  • [ ] ShowEncryptAlgorithmPluginsResultRowBuilderTest
  • [x] ShowEncryptRuleExecutorTest
  • [ ] AlterEncryptRuleExecutorTest
  • [ ] CreateEncryptRuleExecutorTest
  • [ ] DropEncryptRuleExecutorTest
  • [x] ShowMaskRuleExecutorTest
  • [ ] AlterMaskRuleExecutorTest
  • [ ] CreateMaskRuleExecutorTest
  • [ ] DropMaskRuleExecutorTest
  • [ ] ShowReadQueryLoadBalanceAlgorithmPluginsResultRowBuilderTest
  • [x] ShowReadwriteSplittingRuleExecutorTest
  • [ ] ShowStatusFromReadwriteSplittingRulesExecutorTest
  • [x] ShowDefaultShadowAlgorithmExecutorTest
  • [x] ShowShadowAlgorithmsExecutorTest
  • [x] ShowShadowRuleExecutorTest
  • [x] ShowShadowTableRulesExecutorTest
  • [ ] ShowDefaultShardingStrategyExecutorTest
  • [x] ShowShardingAlgorithmExecutorTest
  • [ ] ShowShardingAlgorithmPluginsResultRowBuilderTest
  • [x] ShowShardingAuditorsExecutorTest
  • [x] ShowShardingKeyGeneratorExecutorTest
  • [x] ShowShardingTableNodesExecutorTest
  • [x] ShowShardingTableReferenceRuleExecutorTest
  • [x] ShowShardingTableRuleExecutorTest
  • [x] ShowShardingTableRulesUsedAlgorithmExecutorTest
  • [x] ShowShardingTableRulesUsedAuditorExecutorTest
  • [x] ShowShardingTableRulesUsedKeyGeneratorExecutorTest
  • [x] ShowUnusedShardingAlgorithmsExecutorTest
  • [x] ShowUnusedShardingAuditorsExecutorTest
  • [x] ShowUnusedShardingKeyGeneratorExecutorTest

terrymanu avatar Feb 09 '24 16:02 terrymanu

hi, @terrymanu, I would like to get involved in the open source community starting with this issue

zzzk1 avatar Feb 14 '24 13:02 zzzk1

hi, @terrymanu, I would like to get involved in the open source community starting with this issue

@zzzk1 Welcome onboard!

terrymanu avatar Feb 14 '24 14:02 terrymanu

@terrymanu How are we going to refactor AlterGlobalClockRuleExecutoreTest? I am asking this because in current test case we are using the method of buildToBeAlteredRuleConfiguration(sqlStatement). But the class DistSQLQueryExecuteEngine does not have any equivalent method. Also, I cannot use the execute query because the call to TypedSPI method fails. I know I am not able to put things correctly but it is because I am very much new to the project.

VishalMCF avatar Feb 24 '24 13:02 VishalMCF

DistSQLUpdateExecuteEngine

@VishalMCF pls look DistSQLUpdateExecuteEngine

zzzk1 avatar Feb 24 '24 13:02 zzzk1

@zzzk1 Thank you so much. Are you still working on the issue? If yes can I just create a PR for AlterGlobalClockRuleExecutoreTest test?

VishalMCF avatar Feb 24 '24 13:02 VishalMCF

@zzzk1 Thank you so much. Are you still working on the issue? If yes can I just create a PR for AlterGlobalClockRuleExecutoreTest test?

sure, just go ahead!

zzzk1 avatar Feb 24 '24 14:02 zzzk1

@zzzk1 you can go ahead.It is still very confusing actually.

VishalMCF avatar Feb 24 '24 14:02 VishalMCF

Hi @zzzk1 , Please note that some DistSQLExecutor are related to database rules, but were processed as global rules in the test. I fixed them in #30303.

RaigorJiang avatar Feb 26 '24 15:02 RaigorJiang

Hi @terrymanu , I would like take this issue, if it's not done

Mykiiii avatar Mar 14 '24 19:03 Mykiiii

There hasn't been any activity on this issue recently, and in order to prioritize active issues, it will be marked as stale.

github-actions[bot] avatar Apr 13 '24 20:04 github-actions[bot]

Hi @terrymanu , I would like take this issue, if it's not done

@Mykiiii Sorry to reply late, please go ahead

terrymanu avatar Apr 16 '24 16:04 terrymanu

hi @terrymanu, Are we intentionally closing this issue? I noticed there are incomplete tasks remaining in this issue. Thanks

(edited) It looks like the issue was closed because I flagged the latest pull request as Fixes instead of Ref

ilyasahsan123 avatar May 11 '24 00:05 ilyasahsan123

hi @terrymanu, Are we intentionally closing this issue? I noticed there are incomplete tasks remaining in this issue. Thanks

(edited) It looks like the issue was closed because I flagged the latest pull request as Fixes instead of Ref

The issue was closed automatically due to the pull request association. However, this issue has not been fully resolved, and the unchecked checkboxes still require further modifications.

terrymanu avatar Jul 17 '24 04:07 terrymanu