[#12048] Migrate E2E Tests for InstructorStudentActivityLogsPage#13450
Conversation
|
Hi @WeeJean, thank you for your interest in contributing to TEAMMATES!
Please address the above before we proceed to review your PR. |
samuelfangjw
added
the
c.Task
DhiraPT
changed the title
There was a problem hiding this comment.
Pull request overview
This PR migrates the E2E test for the Instructor Student Activity Logs page from the legacy datastore to SQL. It includes test data migration to SQL-compatible format, updates to the E2E test class, and important fixes to the page object.
Changes:
- Migrated test data from datastore format to SQL entities with proper UUIDs and foreign key relationships
- Updated E2E test to use SQL entities (Course, Student, FeedbackSession, etc.) instead of datastore attributes
- Fixed a critical typo in
InstructorStudentActivityLogsPage.javawheresetLogsFromDateTimewas incorrectly setting the "To" timepicker instead of the "From" timepicker - Added helper methods
getLogsOutputText()andwaitForLogsToLoad()to the page object for better abstraction
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| testng-unstable-e2e-sql.xml | Added the SQL version of InstructorStudentActivityLogsPageE2ETest to the unstable SQL test suite |
| InstructorStudentActivityLogsPageE2ETestSql.json | Complete test data migration with SQL-compatible structure including UUIDs, accounts, sections, and teams |
| InstructorStudentActivityLogsPage.java | Fixed critical bug in setLogsFromDateTime and added helper methods for test assertions |
| InstructorStudentActivityLogsPageE2ETest.java | New SQL version of the E2E test with updated entity references and enhanced date/time filtering |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/e2e/java/teammates/e2e/cases/sql/InstructorStudentActivityLogsPageE2ETest.java
Outdated
Show resolved
Hide resolved
src/e2e/java/teammates/e2e/cases/sql/InstructorStudentActivityLogsPageE2ETest.java
Outdated
Show resolved
Hide resolved
src/e2e/java/teammates/e2e/cases/sql/InstructorStudentActivityLogsPageE2ETest.java
Outdated
Show resolved
Hide resolved
src/e2e/java/teammates/e2e/cases/sql/InstructorStudentActivityLogsPageE2ETest.java
Show resolved
Hide resolved
…m:WeeJean/teammates into migrate-InstructorStudentActivityLogsPage
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Student student = sqlLogic.getStudent(studentId); | ||
| FeedbackSession feedbackSession = sqlLogic.getFeedbackSession(fsId); | ||
|
|
||
| FeedbackSessionLog feedbacksessionlog = new FeedbackSessionLog(student, feedbackSession, |
There was a problem hiding this comment.
The variable name feedbacksessionlog doesn't follow Java camelCase naming conventions. It should be feedbackSessionLog to maintain consistency with the rest of the codebase, where variables of type FeedbackSessionLog are named using proper camelCase (similar to how student and feedbackSession are named on lines 59-60).
mingyuancode
left a comment
mingyuancode
left a comment
There was a problem hiding this comment.
Fix the copilot n my comment and we should be good to go
| FeedbackSessionLog feedbacksessionlog = new FeedbackSessionLog(student, feedbackSession, | ||
| convertedFslType, Instant.now()); | ||
|
|
||
| // Necessary to assist local testing. For production usage, this will be a no-op. |
There was a problem hiding this comment.
Remove this comment here too, it doesnt apply any more
| * Gets the expected number of submissions for a feedback session. | ||
| * | ||
| * <br>Preconditions: <br> | ||
| * <br>Preconditions: <br/> |
There was a problem hiding this comment.
is it required to add the / ?
There was a problem hiding this comment.
While not strictly required by the Java compiler, adding the / is highly recommended to ensure the Javadoc is well-formed XML and to prevent potential build warnings from the documentation tool. This is also to ensure consistency with the
usage in multiple instances in this file before.
4 participants
Overview
This PR migrates the testing infrastructure for the Instructor Student Activity Logs page to the SQL path. It includes updates to the underlying test data (JSON) and the E2E test class to ensure compatibility with the SQL-migrated environment.
Changes
JSON Migration: Migrated
InstructorStudentActivityLogsPageE2ETest.jsonto a SQL-compatible format.E2E Test Updates: Refactored
InstructorStudentActivityLogsPageE2ETest.javato work with the SQL page objects.Page Object Updates
setLogsFromDateTimetypo: Corrected a critical bug inInstructorStudentActivityLogsPage.javawhere the "From" date setter was erroneously overwriting the "To" timepicker. This was causing search ranges to be invalid in SQL-strict environments.getLogsOutputText(): Introduced a helper method to retrieve the raw text from the logs output for easier and more robust assertions.waitForLogsToLoad(): Abstracted the SeleniumwaitForElementPresencelogic into the Page Object to comply with project architecture and keep Selenium imports out of test cases.