Skip to main content
Code Review

Return to Answer

added 1 character in body
Source Link
Ingo Bürk
  • 1k
  • 7
  • 12
public void create_Created(){

This violates Java coding standards. In Java, the convention is camelCase, not snake_case. On top of that, the name create_Created is utterly meaningless. What is that supposed to mean? Give your tests names describing what they test!

@Test
public void create_NoCountryAssociation_ExceptionThrown(){
 exception.expect(ConstraintViolationException.class);

It's good that you are using the ExpectedException rule, but you are using it wrong. Don't expect the exception until right before the call that you expect to trigger the exception – test setup etc. needs to be done before!

Some of your tests don't seem to set up the data you are using for assertions. For your test cases to be independent, you should create the entry in the database from the test and then assert that the DAO can read this value (or do whatever you're testing). Don't rely on what's in the database – it's a messy road to go down because your tests will start depending on each other, potentially causing what we call blinking tests (sometimes thethey pass, sometimes they fail).

Common test teardowns also belong in a @After method rather than being done in every test individually.

public void create_Created(){

This violates Java coding standards. In Java, the convention is camelCase, not snake_case. On top of that, the name create_Created is utterly meaningless. What is that supposed to mean? Give your tests names describing what they test!

@Test
public void create_NoCountryAssociation_ExceptionThrown(){
 exception.expect(ConstraintViolationException.class);

It's good that you are using the ExpectedException rule, but you are using it wrong. Don't expect the exception until right before the call that you expect to trigger the exception – test setup etc. needs to be done before!

Some of your tests don't seem to set up the data you are using for assertions. For your test cases to be independent, you should create the entry in the database from the test and then assert that the DAO can read this value (or do whatever you're testing). Don't rely on what's in the database – it's a messy road to go down because your tests will start depending on each other, potentially causing what we call blinking tests (sometimes the pass, sometimes they fail).

Common test teardowns also belong in a @After method rather than being done in every test individually.

public void create_Created(){

This violates Java coding standards. In Java, the convention is camelCase, not snake_case. On top of that, the name create_Created is utterly meaningless. What is that supposed to mean? Give your tests names describing what they test!

@Test
public void create_NoCountryAssociation_ExceptionThrown(){
 exception.expect(ConstraintViolationException.class);

It's good that you are using the ExpectedException rule, but you are using it wrong. Don't expect the exception until right before the call that you expect to trigger the exception – test setup etc. needs to be done before!

Some of your tests don't seem to set up the data you are using for assertions. For your test cases to be independent, you should create the entry in the database from the test and then assert that the DAO can read this value (or do whatever you're testing). Don't rely on what's in the database – it's a messy road to go down because your tests will start depending on each other, potentially causing what we call blinking tests (sometimes they pass, sometimes they fail).

Common test teardowns also belong in a @After method rather than being done in every test individually.

Source Link
Ingo Bürk
  • 1k
  • 7
  • 12
public void create_Created(){

This violates Java coding standards. In Java, the convention is camelCase, not snake_case. On top of that, the name create_Created is utterly meaningless. What is that supposed to mean? Give your tests names describing what they test!

@Test
public void create_NoCountryAssociation_ExceptionThrown(){
 exception.expect(ConstraintViolationException.class);

It's good that you are using the ExpectedException rule, but you are using it wrong. Don't expect the exception until right before the call that you expect to trigger the exception – test setup etc. needs to be done before!

Some of your tests don't seem to set up the data you are using for assertions. For your test cases to be independent, you should create the entry in the database from the test and then assert that the DAO can read this value (or do whatever you're testing). Don't rely on what's in the database – it's a messy road to go down because your tests will start depending on each other, potentially causing what we call blinking tests (sometimes the pass, sometimes they fail).

Common test teardowns also belong in a @After method rather than being done in every test individually.

lang-java

AltStyle によって変換されたページ (->オリジナル) /