Skip to content

Committing the files sent by Elizabeta#2

Open
aleksandermilchev wants to merge 1 commit intomainfrom
ElizabetaPetkovaQATask
Open

Committing the files sent by Elizabeta#2
aleksandermilchev wants to merge 1 commit intomainfrom
ElizabetaPetkovaQATask

Conversation

@aleksandermilchev
Copy link

Copy link
Author

@aleksandermilchev aleksandermilchev left a comment

Choose a reason for hiding this comment

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

Generally the basic cases are implemented and are working, but there are several important cases which are skipped. Having in mind that the task is to test a simple calculator it is not acceptable to skip the "divide by zero" scenario. Also only successful cases are tested, what if we pass an unimplemented value for parameter "mathOperator" of the method Calculate().

public class CalculationResult
{
public bool isSuccessful { get; set; }
public bool Successful { get; set; }
Copy link
Author

Choose a reason for hiding this comment

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

The tests should not change the existing code!

{
case MathOperatorType.Add:
result.DecimalResult = a + b;
result.isSuccessful = true;
Copy link
Author

Choose a reason for hiding this comment

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

Same here


#basic scenarios

Scenario: Add two numbers
Copy link
Author

Choose a reason for hiding this comment

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

I don't see a case testing division by zero.
Can't see a case testing the IsSuccessful = false.
Nothing for the exception logging.

Scenario: Substract two numbers
Given a digit is 40
And b digit is 20
When Substract them
Copy link
Author

Choose a reason for hiding this comment

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

Not all possible values of the MathOperatorType enum are tested, "Power" is skipped.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant