Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Statistics #65

Merged
merged 10 commits into from
Sep 7, 2018
Merged

Statistics #65

merged 10 commits into from
Sep 7, 2018

Conversation

TihomirovPavel
Copy link
Contributor

Checklist

  • I've run mvn test from the root directory to see all new and existing tests pass
  • I've followed code style and run and ensured the code style is valid
  • I've created new tests if necessary.

Motivation and Context

#63 #59

Description

just small final fixes, tests

liptons333
liptons333 previously approved these changes Sep 7, 2018
}
}
var obj = "<table>" +
Copy link
Contributor

Choose a reason for hiding this comment

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

Should have a clearer name, such as statisticsTable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, will do


@Test
@DisplayName("test setStatusAchieved: throws ValidationException wrong user")
void testSetStatusAchievedThrowsExceptionIfWrongUser() {
Copy link
Contributor

@olegvm olegvm Sep 7, 2018

Choose a reason for hiding this comment

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

For such long names, underscores can be used to separate parts of names:
testSetStatusAchieved_ThrowsExceptionIfWrongUser
I.e. "test" + method name that is being tested + underscore + extra part (+ underscore + more parts...).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok. will remember should i do it now?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, better do it, should be quick to do.


@Test
@DisplayName("test setStatusAchieved: changes status")
void test_SetStatusAchieved() {
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to put the underscore right after "test" - testSetStatusAchieved is OK.

@olegvm olegvm merged commit feb81fd into master Sep 7, 2018
@olegvm olegvm deleted the statistics branch September 7, 2018 11:39
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.

4 participants