unloggedio / unlogged-sdk

Unlogged SDK for recording code execution
https://unlogged.io
Apache License 2.0
152 stars 16 forks source link

Test generation for method without any arguments is not generating all class level variables in that #55

Closed tushar886 closed 1 month ago

tushar886 commented 1 month ago

Describe the bug

For the cases where the method is not having any arguments and using class level arguments only. COnsider the case of struts where the ACtion class code is like this 👍

`File Name : Employee.java

@Authorized(action = "commonData", permission = "read") public void getEmpDetailDataList() throws Exception { try { if (employeeIdList == null) employeeIdList = new ArrayList<>();

        List<EntitySecurityLevel> employeeDataList;
        if (!sma) {
            List<Integer> filteredEmpIDList = empManager.getApplicableEmpListForMidMonthDate(employeeIdList, date,
                    deptID);
            if (filteredEmpIDList != null && !filteredEmpIDList.isEmpty()) {
                employeeDataList = empManager.getEmpDetailDataList(filteredEmpIDList, date, date, securityLevelID);
            } else {
                out.println(JSONUtils.getValidationJSON(messages.getValue(MessageCodes.INCORRECT_SECURITY_DATE)));
                return;
            }

            employeeDataList = empManager.getEmpDetailDataList(employeeIdList, date, date, deptID);
        } else {
            employeeDataList = empDeptMgr.getSMASecurityIDForSmaIDs(employeeIdList, date, deptID);
        }

        // data for treegrid
        String dataXML = gridXMLGenerator.generateTreeGridDataXML(
                GridType.searchConfig.toString(), new ArrayList<>(employeeDataList), request,
                response);

        // layout data
        String layoutXML = gridXMLGenerator.generateTreeGridLayoutXML(
                GridType.searchConfig.toString(), true, false);

        Map<String, String> result = new HashMap<>();
        result.put("dataXML", dataXML);
        result.put("layoutXML", layoutXML);
        response.getWriter().println(JSONUtils.getSuccessJSON(result));
    } catch (BusinessException e) {
        response.getWriter().println(JSONUtils.getValidationJSON(e.getMessage()));
    }
}`

Here we can see that method in itself does not have any arguments rather its using all class level arguments.

When test generation of such a method is happening there all variables are not getting mocked but rather few are skipped.

Mocked generated test of such example

`
public final class TestEmployeeV {

private EmployeeManager empManager;

private EmployeeDeptManager empDeptMgr;

private Integer integerVar;

private Boolean booleanVar;

private ObjectMapper objectMapper = new ObjectMapper();

@BeforeEach
public void setup() throws Exception {
    empDeptMgr = Mockito.mock(EmployeeDeptManager.class);
    emp = new Employee();
    integerVar = Integer.valueOf(1);
    booleanVar = Boolean.valueOf(true);
    injectField(emp, "empDeptMgr", empDeptMgr);
    injectField(emp, "integerVar", integerVar);
    injectField(emp, "booleanVar", booleanVar);
    gridXMLGenerator = Mockito.mock(TreeGridXMLGenerator.class);
    servletResponse = Mockito.mock(ServletResponse.class);
    request = Mockito.mock(HttpServletRequest.class);
    empManager = Mockito.mock(EmployeeManager.class);
    booleanVar = Boolean.valueOf(false);
    injectField(emp, "gridXMLGenerator", gridXMLGenerator);
    injectField(emp, "servletResponse", servletResponse);
    injectField(emp, "request", request);
    injectField(emp, "empManager", empManager);
    integerVar = new Integer();
    booleanVar = new Boolean();
}

@Test
public void testchangeEmpDetails() throws Exception {
    // Test candidate method [getEmpDetailDataList] [450657,11] - took 4881ms
    try {
        // this is going to throw exception <java.lang.String>
        emp.getEmpDetailDataList();
        Assertions.assertFalse(true);
    } catch (Exception e) {
        Assertions.assertEquals(String.class, e.getClass());
    }
}`

Here we can see the fields like employeeDataList and date are missing from test case generated

Reproduction steps

  1. Write a method as defined above in any java file
  2. Run the live use case via unlogged
  3. Let the timeline capture the data
  4. Click on generate junit test and observer that the internal variables are missing ...

Expected behavior

All related variables within scope of that method should be mocked and generated

Additional context

No response

kingkong-AI commented 1 month ago

Tried to reproduce the issue using this class:

package org.unlogged.demo.service;

import java.util.ArrayList;
import java.util.List;
import java.util.Map;

public class SimpleService {
    private List<String> dataList;
    private String importantString;
    private int importantNumber;
    private double importantDouble;
    private Map<Integer, String> importantMap;

    public SimpleService(Map<Integer, String> importantMap) {
        this.dataList = new ArrayList<>();
        this.importantString = "";
        this.importantNumber = 0;
        this.importantDouble = 0.0;
        this.importantMap = importantMap;
    }

        public List<String> processData() {
            dataList = new ArrayList<>();
        dataList.add("TestString");
        importantDouble = 5.5;

        importantNumber = (int) Math.round(importantDouble);
        importantString = "hello";
        for (int i = 0; i < importantNumber; i++) {
            dataList.add(importantString + i);
            importantMap.put(i,importantString + i);
        }
       return dataList;
    }
}

The generated test case looks like:

package org.unlogged.demo.service;

import static io.unlogged.UnloggedTestUtils.*;
import static org.mockito.ArgumentMatchers.*;

import com.fasterxml.jackson.databind.ObjectMapper;
import java.lang.Double;
import java.lang.Exception;
import java.lang.String;
import java.util.List;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;

public final class TestSimpleServiceV {
  private SimpleService simpleService;

  private J jVar;

  private Double doubleValueVar;

  private ObjectMapper objectMapper = new ObjectMapper();

  @Before
  public void setup() throws Exception {
    simpleService = new SimpleService();
    jVar = new J();
    doubleValueVar = new Double();
    injectField(simpleService, "jVar", jVar);
    injectField(simpleService, "doubleValueVar", doubleValueVar);
  }

  @Test
  public void processData() throws Exception {
    // Test candidate method [processData] [451,1] - took 3ms
    List<String> list = simpleService.processData();
    List listExpected = objectMapper.readValue("[\"TestString\",\"hello0\",\"hello1\",\"hello2\",\"hello3\",\"hello4\",\"hello5\"]", List.class);
    Assert.assertEquals(listExpected, list);
  }
}

Multiple Issues are visible:

  1. Skipping out a lot of class level fields.
  2. For class level Double it is doing new Double(); which is an invalid constructor.
  3. Creating random data types. For example: J in this case.

All these need to be fixed in Test generation.

kingkong-AI commented 1 month ago

Class used:

package org.unlogged.demo.controller;

import java.util.ArrayList;
import java.util.List;
import java.util.Map;

public class SampleService {
    private List<String> dataList;
    private String importantString;
    private int importantNumber;
    private double importantDouble;
    private Map<Integer, String> importantMap;

    public SampleService(Map<Integer, String> importantMap) {
        this.dataList = new ArrayList<>();
        this.importantString = "";
        this.importantNumber = 0;
        this.importantDouble = 0.0;
        this.importantMap = importantMap;
    }

    public List<String> processData() {
        dataList = new ArrayList<>();
        dataList.add("TestString");
        importantDouble = 5.5;

        importantNumber = (int) Math.round(importantDouble);
        importantString = "hello";
        for (int i = 0; i < importantNumber; i++) {
            dataList.add(importantString + i);
            importantMap.put(i,importantString + i);
        }
        return dataList;
    }
}

The final test case generated:

package org.unlogged.demo.controller;

import static io.unlogged.UnloggedTestUtils.*;
import static org.mockito.ArgumentMatchers.*;

import com.fasterxml.jackson.databind.ObjectMapper;
import java.lang.Exception;
import java.lang.Integer;
import java.lang.String;
import java.util.HashMap;
import java.util.List;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

public final class TestSampleServiceV {
  private SampleService sampleService;

  private HashMap<Integer, String> importantMap;

  private ObjectMapper objectMapper = new ObjectMapper();

  @BeforeEach
  public void setup() throws Exception {
    importantMap = new HashMap();
    sampleService = new SampleService(importantMap);
  }

  @Test
  public void testMethodProcessData() throws Exception {
    // Test candidate method [processData] [483,1] - took 1ms
    List<String> list = sampleService.processData();
    List listExpected = objectMapper.readValue("[\"TestString\",\"hello0\",\"hello1\",\"hello2\",\"hello3\",\"hello4\",\"hello5\"]", List.class);
    Assertions.assertEquals(listExpected, list);
  }
}

Replacing the single argument constructor with:

    public SampleService(List<String> dataList, String importantString, int importantNumber, double importantDouble, Map<Integer, String> importantMap) {
        this.dataList = dataList;
        this.importantString = importantString;
        this.importantNumber = importantNumber;
        this.importantDouble = importantDouble;
        this.importantMap = importantMap;
    }

The final test case generated is:

package org.unlogged.demo.controller;

import static io.unlogged.UnloggedTestUtils.*;
import static org.mockito.ArgumentMatchers.*;

import com.fasterxml.jackson.databind.ObjectMapper;
import java.lang.Exception;
import java.lang.Integer;
import java.lang.String;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

public final class TestSampleServiceV {
  private SampleService sampleService;

  private ArrayList<String> dataList;

  private HashMap<Integer, String> importantMap;

  private ObjectMapper objectMapper = new ObjectMapper();

  @BeforeEach
  public void setup() throws Exception {
    importantMap = new HashMap();
    dataList = new ArrayList();
    String importantString = "";
    sampleService = new SampleService(dataList, importantString, 0, 0, importantMap);
  }

  @Test
  public void testMethodProcessData() throws Exception {
    // Test candidate method [processData] [455,1] - took 2ms
    List<String> list = sampleService.processData();
    List listExpected = objectMapper.readValue("[\"TestString\",\"hello0\",\"hello1\",\"hello2\",\"hello3\",\"hello4\",\"hello5\"]", List.class);
    Assertions.assertEquals(listExpected, list);
  }
}

Here an improvement could be to initialize double with 0.0 and not just 0 for clarity in the test case.