Skip to content

Fix CLI test warnings#1450

Merged
abhishekkumams merged 1 commit intomainfrom
dev/abhishekuma/fix_cli_test_warning
Apr 25, 2023
Merged

Fix CLI test warnings#1450
abhishekkumams merged 1 commit intomainfrom
dev/abhishekuma/fix_cli_test_warning

Conversation

@abhishekkumams
Copy link
Contributor

@abhishekkumams abhishekkumams commented Apr 25, 2023 •
edited
Loading

Why make this change?

  • Some unit Tests were complaining about wrong Signature of the Class Initialization Test Setup method, due to which they were not getting discovered.

What is this change?

  • When declaring ClassInitialize attribute on a method, the method has to be static, public, void and should take a single parameter of type TestContext.
  • I added a parameter of type TestContext to fix.
  • Performance Improvement by using ClassInitialize than TestInitialize.

How was this tested?

  • Passing existing Unit Tests
  • No warnings

@abhishekkumams abhishekkumams self-assigned this Apr 25, 2023
seantleonard
seantleonard approved these changes Apr 25, 2023
View reviewed changes
Copy link
Contributor

@seantleonard seantleonard left a comment

Choose a reason for hiding this comment

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

Thank you for addressing! also looks like additional efficiency by using Classinitialize over testinitialize.

@abhishekkumams abhishekkumams enabled auto-merge (squash) April 25, 2023 04:06
@abhishekkumams abhishekkumams merged commit e29a3f0 into main Apr 25, 2023
@abhishekkumams abhishekkumams deleted the dev/abhishekuma/fix_cli_test_warning branch April 25, 2023 05:05
@abhishekkumams abhishekkumams added refactor improvement Let's make this better labels Apr 25, 2023
@abhishekkumams abhishekkumams added this to the Apr2023 milestone Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@seantleonard seantleonard seantleonard approved these changes

@severussundar severussundar severussundar approved these changes

@mbhaskar mbhaskar Awaiting requested review from mbhaskar

@Aniruddh25 Aniruddh25 Awaiting requested review from Aniruddh25 Aniruddh25 is a code owner

@JelteF JelteF Awaiting requested review from JelteF

@Mathos1432 Mathos1432 Awaiting requested review from Mathos1432

@gledis69 gledis69 Awaiting requested review from gledis69

@aaronburtle aaronburtle Awaiting requested review from aaronburtle aaronburtle is a code owner

@milismsft milismsft Awaiting requested review from milismsft

@tarazou9 tarazou9 Awaiting requested review from tarazou9

@ayush3797 ayush3797 Awaiting requested review from ayush3797

@aaronpowell aaronpowell Awaiting requested review from aaronpowell

@ravishetye ravishetye Awaiting requested review from ravishetye

Assignees

@abhishekkumams abhishekkumams

Labels

improvement Let's make this better refactor

Projects

Data API builder
Status: Done

Milestone

0.7

Development

Successfully merging this pull request may close these issues.

3 participants

@abhishekkumams @seantleonard @severussundar