best practice: how to set config file at runtime for tests

Question

As a homework exercise (I'm a beginner) I had to write a Java program which accesses a database (PostgreSQL). The program can, e.g., insert new users, increment some fields etc. and I have methods such as: addUser(User t), deleteUser(User t) and so on.

I also have written test methods using junit5. For the tests I use a 'test' database, separate from the 'work' one. The coordinates to open the two databases (database name, password etc.) are stored in two files called config.properties and config.test.properties, which are selected at runtime.

What I have in place now is something along these lines, using a boolean flag variable:

    public class UserDao {
    public boolean isTestMode = false; 

    public Connection getConnection() {

        if (this.isTestMode) {
            input = cl.getResourceAsStream("config.test.properties");
        } else {
            input = cl.getResourceAsStream("config.properties");
        }
        ...
    }
}

In my test methods I then set the flag like this:

        void testAddUser() {
        UserDao dao = new UserDao();
        dao.isTestMode = true;
        ...
        }

In regular, non-test method I don't set isTestMode, which therefore stays to its default (false) value and config.properties is used. My approach works, but my instructor told me it is bad practice to do things like this and I should change it, for example (he suggested) doing a dependency injection. I'm not too sure how to proceed. I could make configFilename a class variable and add to the class UserDao a new constructor which accepts a filename, like this:

public class UserDao {
private String configFilename = "config.properties"; 

public UserDao() {
}

public UserDao(String filename) {
    this();
    this.configFilename = filename;
}

public Connection getConnection() {

    input = cl.getResourceAsStream(this.configFilename);
    ...
    }

}

Then in the test methods I use the new construtor UserDao("config.test.properties")

A (in my view a better) variant is to introduce a constructor which accepts a boolean isTestMode and sets configFilename accordingly (I don't need nor want the flexibility of specifying any filename in the constructor). But essentially this is the same as my original approach, which I was told to change. Also, there no dependency injection there... what is it best practice in such cases? Any suggestion would be welcome!

See also https://stackoverflow.com/questions/24231773/specifying-a-custom-log4j-properties-file-for-all-of-junit-tests-run-from-eclips — Raedwald, Jun 12 '19 at 10:35

score 0 · Answer 1 · answered Jun 12 '19 at 08:37

In application use default constructor when creating an instance of UserDao, in junit pass name of file:

new UserDao("config.test.properties");

private final String configFilename; 

public UserDao() {
  this("config.properties");
}

public UserDao(String filename) {
    this.configFilename = filename;
}

score 0 · Accepted Answer · answered Jun 12 '19 at 09:08

0

Passing parameter can be seen as a trivial dependency injection.

Concerning your java: when you set a default value for configFilename, you can see that as a convention that you use in your application.

Your class does not need that. And if you avoid this convention, you got immutability for free. For example, you can do :

    public class UserDao {


    private final String configFilename;

    public UserDao(String filename) {
        this.configFilename = filename;
    }

    public Connection getConnection() {

        input = cl.getResourceAsStream(this.configFilename);
    ...
    }

}

UserDao can be used by your test classes or your main classes the same way.

With your solution, you may deliver code that will never run in production (the branch where isTestMode is true), and that's not a good practice. This code could be seen as dead code in production.

answered Jun 12 '19 at 09:08

Vincent Biragnet

2,950
15
22

thanks!!! That's a very clear explanation. I didn't consider immutability as one of the desiderata (I understand now that it is). I adopted your suggestion. – Lorents Jun 12 '19 at 09:46
You're welcome. Other things to note: 1) your stream (`input` parameter) should be in a `try` to be sure it will be closed properly. 2) Every time you use `getConnection`, you will read the configuration file. If it's an issue in your case, you have to find a way to read them only once and e.g. pass the relevant info in an dedicated object in the constructor. – Vincent Biragnet Jun 12 '19 at 10:12
thanks! Regarding 1) : yes, the stream is in a `try/catch/finally` block and I make sure I close the connection. 2) For my exercise I think it's okay to re-read the configuration file each time... In an actual application I understand it may be undesirable. In fact, in a previous version of my exercise I had in the class `UserDao` two methods `initializeDao()` and `disconnectDao()`. The former opened a new connection, the second closed it. In the present version each method such as `addUser` opens and closes its own connection by itself. Do you think this second approach is preferable? – Lorents Jun 12 '19 at 10:52
Really depends on the rest of the app... If you have a lot of classes like that, you end up with a code full of "technical" call to disconnect and initialize... It seems better to me to handle all this technical stuffs inside the object and avoid polluting the caller code with lots of technical function call on the object. – Vincent Biragnet Jun 12 '19 at 12:12

best practice: how to set config file at runtime for tests

2 Answers2