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

[#12048] Migrate tests for CalculateUsageStatisticsAction #13247

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
package teammates.sqlui.webapi;

import static org.mockito.ArgumentMatchers.isA;
import static org.mockito.Mockito.when;

import java.time.Instant;
import java.time.temporal.ChronoUnit;
import java.util.List;

import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;

import teammates.common.datatransfer.attributes.UsageStatisticsAttributes;
import teammates.common.util.Const;
import teammates.common.util.TimeHelper;
import teammates.storage.sqlentity.UsageStatistics;
import teammates.ui.webapi.CalculateUsageStatisticsAction;

/**
* SUT: {@link CalculateUsageStatisticsAction}.
*/
public class CalculateUsageStatisticsActionTest extends BaseActionTest<CalculateUsageStatisticsAction> {
private static final int NUMBER_OF_RESPONSES = 2;
private static final int NUMBER_OF_COURSES = 2;
private static final int NUMBER_OF_STUDENTS = 2;
private static final int NUMBER_OF_INSTRUCTORS = 2;
private static final int NUMBER_OF_ACCOUNT_REQUESTS = 2;
private static final int COLLECTION_TIME_PERIOD = 60;
Instant endTime = TimeHelper.getInstantNearestHourBefore(Instant.now());
Instant startTime = endTime.minus(COLLECTION_TIME_PERIOD, ChronoUnit.MINUTES);
UsageStatistics testUsageStatistics;
UsageStatisticsAttributes testUsageStatisticsAttributes;
Comment on lines +29 to +32
Copy link
Contributor

Choose a reason for hiding this comment

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

let's make these attributes private for consistency


@Override
protected String getActionUri() {
return Const.CronJobURIs.AUTOMATED_USAGE_STATISTICS_COLLECTION;
}

@Override
protected String getRequestMethod() {
return GET;
}

@Test
void testAccessControl_admin_canAccess() {
verifyCanAccess();
}

@Test
void testAccessControl_maintainers_cannotAccess() {
logoutUser();
loginAsMaintainer();
verifyCannotAccess();
}

@Test
void testAccessControl_instructor_cannotAccess() {
logoutUser();
loginAsInstructor(Const.ParamsNames.INSTRUCTOR_ID);
verifyCannotAccess();
}

@Test
void testAccessControl_student_cannotAccess() {
logoutUser();
loginAsStudent(Const.ParamsNames.STUDENT_ID);
verifyCannotAccess();
}

@Test
void testAccessControl_loggedOut_cannotAccess() {
logoutUser();
verifyCannotAccess();
}

@Test
void testAccessControl_unregistered_cannotAccess() {
logoutUser();
loginAsUnregistered(Const.ParamsNames.USER_ID);
verifyCannotAccess();
}

@BeforeMethod
void setUp() {
loginAsAdmin();
testUsageStatistics = new UsageStatistics(
startTime,
COLLECTION_TIME_PERIOD,
NUMBER_OF_RESPONSES,
NUMBER_OF_COURSES,
NUMBER_OF_STUDENTS,
NUMBER_OF_INSTRUCTORS,
NUMBER_OF_ACCOUNT_REQUESTS,
0,
0);
Comment on lines +86 to +95
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can create a getTypicalUsageStatistics() method in BaseTestCase.java with all of these values/attributes over there for extensibility.

testUsageStatisticsAttributes =
UsageStatisticsAttributes.builder(startTime, COLLECTION_TIME_PERIOD)
.withNumResponses(NUMBER_OF_RESPONSES)
.withNumCourses(NUMBER_OF_COURSES)
.withNumStudents(NUMBER_OF_STUDENTS)
.withNumInstructors(NUMBER_OF_INSTRUCTORS)
.withNumAccountRequests(NUMBER_OF_ACCOUNT_REQUESTS)
.build();
Comment on lines +96 to +103
Copy link
Contributor

Choose a reason for hiding this comment

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

In UsageStatisticsAttributes.java, there is a valueOf() method that takes in a teammates.storage.entity.UsageStatistics and creates a UsageStatisticsAttributes with the same attributes. It's quite similar to what we're trying to do here

I think we can create another valueOf (Overloading) for the SQL version of UsageStatistics. To resolve the class name conflicts, the new method's parameter type can be teammates.sqlstorage.entity.UsageStatistics (No need to import). The old method can remain.

@InfinityTwo Let me know what you think about this proposal!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the idea is great, but from what I saw the last time, the use of UsageStatisticsAttributes is only for the old datastore logic. I think once migration is completed, I believe the use for it becomes redundant and it solely uses UsageStatistics in the action, assuming we remember to change it after migration. Right now, it seems like it just concatenates both values together for the output.

I was thinking we could add a comment to remove it eventually but @jasonqiu212 what do you think? I'm cool with either ways.

}

@Test
public void testExecute_normalCase_shouldSucceed() {
when(mockLogic.calculateEntitiesStatisticsForTimeRange(isA(Instant.class), isA(Instant.class)))
.thenReturn(testUsageStatistics);
when(mockDatastoreLogic.calculateEntitiesStatisticsForTimeRange(isA(Instant.class), isA(Instant.class)))
.thenReturn(testUsageStatisticsAttributes);
when(mockLogic.getUsageStatisticsForTimeRange(isA(Instant.class), isA(Instant.class)))
.thenReturn(List.of(testUsageStatistics));

CalculateUsageStatisticsAction action = getAction();
action.execute();

List<UsageStatistics> statsObjects = mockLogic.getUsageStatisticsForTimeRange(
TimeHelper.getInstantDaysOffsetBeforeNow(1L),
TimeHelper.getInstantDaysOffsetFromNow(1L));

// Only check that there is a stats object created.
// Everything else is not predictable.
assertEquals(1, statsObjects.size());

UsageStatistics statsObject = statsObjects.get(0);
assertEquals(COLLECTION_TIME_PERIOD, statsObject.getTimePeriod());

// Note that there is a slim possibility that this assertion may fail, if the hour has changed
// between when the stats was gathered and the line where Instant.now is called.
// However, as the execution happens in milliseconds precision, the risk is too small to justify
// the additional code needed to handle this case.
Instant pastHour = TimeHelper.getInstantNearestHourBefore(Instant.now()).minus(1, ChronoUnit.HOURS);
assertEquals(pastHour, statsObject.getStartTime());

}
}
Loading