Question: Is there anything you could do in order to improve the test? I am using Spring Framework 4, Hibernate 4, JUnit 4. All DAO unit tests inherit from TestDaoSetup class:@ContextConfiguration(locations={        classpath:/config/test/spring.xml,        classpath:/config/test/hibernate.xml})@TransactionConfiguration@Transactionalpublic class TestDaoSetup {}One of unit tests:@RunWith(SpringJUnit4ClassRunner.class)public class CityDaoImplTest extends TestDaoSetup {    @Rule    public ExpectedException exception = ExpectedException.none();    @Autowired    private CityDao cityDao;    @Autowired    private CountryDao countryDao;    @Test    public void create_Created(){        CountryEntity countryEntity = countryDao.find(1L);        CityEntity cityEntity = new CityEntity();        cityEntity.setName(Koice);        cityEntity.setCountryEntity(countryEntity);        cityDao.create(cityEntity);        Assert.assertNotNull(Expected not null value.,cityDao.find(cityEntity.getId()));    }    @Test    public void create_NoCountryAssociation_ExceptionThrown(){        exception.expect(ConstraintViolationException.class);        CityEntity cityEntity = new CityEntity();        cityEntity.setName(Koice);        cityDao.create(cityEntity);        cityDao.flush();    }    @Test    public void create_ExistingNameSameCountry_ExceptionThrown() {        exception.expect(ConstraintViolationException.class);        CountryEntity countryEntity = countryDao.find(1L);        CityEntity cityEntity = new CityEntity();        cityEntity.setName(Bratislava);        cityEntity.setCountryEntity(countryEntity);        cityDao.create(cityEntity);        cityDao.flush();    }    @Test    public void create_ExistingNameDifferentCountry_Created(){        CountryEntity countryEntity = countryDao.find(2L);        CityEntity cityEntity = new CityEntity();        cityEntity.setName(Bratislava);        cityEntity.setCountryEntity(countryEntity);        cityDao.create(cityEntity);        Assert.assertNotNull(Expected not null value., cityDao.find(cityEntity.getId()));    }    @Test    public void update_ExistingNameSameCountry_ExceptionThrown(){        exception.expect(ConstraintViolationException.class);        CityEntity cityEntity = cityDao.find(2L);        cityEntity.setName(Bratislava);        cityDao.update(cityEntity);        cityDao.flush();    }    @Test    public void update_ExistingNameDifferentCountry_Updated(){        CityEntity cityEntity = cityDao.find(3L);        cityEntity.setName(Bratislava);        cityDao.update(cityEntity);        Assert.assertEquals(Expected different value., Bratislava, cityDao.find(3L).getName());    }    @Test    public void delete_NotUsed_Deleted() {        CityEntity cityEntity = cityDao.find(4L);        cityDao.delete(cityEntity);        Assert.assertNull(Expected null value., cityDao.find(cityEntity.getId()));    }    @Test    public void delete_Used_ExceptionThrown(){        exception.expect(ConstraintViolationException.class);        CityEntity cityEntity = cityDao.find(1L);        cityDao.delete(cityEntity);        cityDao.flush();    }}Question: Is there anything you could improve in the test? EDIT:To make test conditions more clear:There are separated configurations, one is specifically for testing purposes.There is database script which prepopulates the test database with test data before tests actually start.Data are the same for each test, because there is a rollback after each test.Test data are not prepopulated in each test class, but they are prepopulated in separate SQL file, which is executed before tests actually start.
Create better Unit test
java;unit testing;junit
External dataMost of your test cases are expected that some records already exist. The details of these records are not visible in the class, which makes them an external dependency. It also makes the test cases harder to understand.A simple fix is to add the necessary test data in the test cases, or in the @Before method.Make assumptions explicitSome of the test cases are hard to understand because they make assumptions that are not exactly obvious, for example:@Testpublic void create_ExistingNameDifferentCountry_Created(){    CountryEntity countryEntity = countryDao.find(2L);    CityEntity cityEntity = new CityEntity();    cityEntity.setName(Bratislava);    cityEntity.setCountryEntity(countryEntity);    cityDao.create(cityEntity);    Assert.assertNotNull(Expected not null value., cityDao.find(cityEntity.getId()));}Here, I suppose you are testing that you can create a city with the same name in a different country. This test case seems to make some assumptions:There is no city named Bratislava in the Country with id=2There is a city name Bratislava in some other CountryIt's better to make these assumptions part of the code, for example:private CountryEntity createNewCountry() {    // TODO: create a new unique country (different from all existing)}@Testpublic void create_ExistingNameDifferentCountry_Created(){    CountryEntity someCountry = createNewCountry();    createCity(Bratislava, someCountry);    CountryEntity anotherCountry = createNewCountry();    CityEntity cityEntity = new CityEntity();    cityEntity.setName(Bratislava);    cityEntity.setCountryEntity(anotherCountry);    cityDao.create(cityEntity);    Assert.assertNotNull(Expected not null value., cityDao.find(cityEntity.getId()));}There are no more meaningless numbers (such as 2L) in this test case, and the assumptions are loud and clear.Another similar example:@Testpublic void update_ExistingNameSameCountry_ExceptionThrown(){    exception.expect(ConstraintViolationException.class);    CityEntity cityEntity = cityDao.find(2L);    cityEntity.setName(Bratislava);    cityDao.update(cityEntity);    cityDao.flush();}This test case seems to assume that the city with id=2 is named Bratislava. Consider this instead:@Testpublic void update_ExistingNameSameCountry_ExceptionThrown(){    exception.expect(ConstraintViolationException.class);    CityEntity cityEntity = cityDao.findSomeCity();    cityEntity.setName(cityEntity.getName());    cityDao.update(cityEntity);    cityDao.flush();}No more hidden assumptions and invisible external data. The city can be any city, if you try to create a new city with the same name as the sample city, the method should throw.Many of your other methods suffer from the same problem. Review them and ask yourself if there are any assumptions that are not written inside the method itself, and make them explicit.Covering all cornersSome of your test cases could be more strict, for example:In create_Created, in addition to checking that you can load the new entry that was created, you could check that:the number of entries in the database got increased by 1Assert.assertEquals(cityEntity, cityDao.find(cityEntity.getId()))Similarly in create_ExistingNameDifferentCountry_CreatedExpected exceptionsHave you considered this instead of the @Rule for expected exceptions:@Test(expected=ConstraintViolationException.class)public void create_NoCountryAssociation_ExceptionThrown(){    // ...}Personally I find this more readable. @Rule is more powerful, because it can match exception messages too, but you're not using that feature anyway. This may be a matter of taste.NamingI think your test case names could be more intuitive, for example:create_city_with_country_works instead of create_Createdcreate_city_without_country_throws instead of NoCountryAssociation_ExceptionThrown...As for camelCase vs underscores vs mixed vs something else, I think it doesn't matter. You can use what you like best. The general naming rules probably don't apply to the test cases the same way as to regular code. Test case names should be specific, identify what is being tested, and as such they tend to be very long.For example, underscores are very similar to spaces, so create_city_without_country_throws is just a little bit harder to read than a sentence. In  createCityWithoutCountryThrows there is no visual separation between the words, which makes it much harder to read than a sentence. At least this is my opinion.The bottom line is, use whatever naming is agreed in your team, or else whatever naming is more readable to you.