[#11878] Merge master into feature (#13011)

* Update chrome driver download link in e2e-testing.md (#12924) * [#12048] Add SQL configuration into build.properties and build-dev.properties (#12917) * Add production config * Remove forgotten host and password * Fix lint --------- Co-authored-by: Zhang Ziqing <69516975+ziqing26@users.noreply.github.com> * [#12048] Add SQL description for postgres config (#12931) * Add production config * Remove forgotten host and password * Fix lint * Address changes, include production_user * Linting * [#12588] Improve test code coverage of core components - ToastComponent (#12916) * add test cases * add test case for isTemplate() --------- Co-authored-by: Cedric Ong <67156011+cedricongjh@users.noreply.github.com> Co-authored-by: Dominic Lim <46486515+domlimm@users.noreply.github.com> * [#12588] Add unit tests to question edit answer form (#12935) * add unit tests to constsum-options-question-edit-answer-form * add unit tests to constsum-options-question-edit-answer-form --------- Co-authored-by: Zhang Ziqing <69516975+ziqing26@users.noreply.github.com> * add delay to task queuer for indexing account request (#12936) Co-authored-by: Nicolas <25302138+NicolasCwy@users.noreply.github.com> * Make account req data migration script rerunnable (#12932) * [#12048] Relax read notif verification for migration verification script (#12937) * Fix account requests with wrong field during seed * Relax account attributes verification * Fix lint errors * Fix order of account request variables * [#12920] Create script to migrate noSQL test data to SQL schema format (#12922) * Add classes to migrate test json data * Add toposort script * Add function to remove foreign key data * Cleanup * WIP * Simplify keys for students and instructors * Fix lint issues * Output SQL JSON in same folder as JSON * Change output file name * Fix bug: wrong jsonkey used * Fix lint error * Make section and team name unique * Set read notification key to be unique * Delete python file * [#12588] Improve test code coverage of core components - ViewResultsPanelComponent (#12918) * add test cases to ViewResultsPanelComponent * fix lint errors --------- Co-authored-by: Dominic Lim <46486515+domlimm@users.noreply.github.com> Co-authored-by: Zhang Ziqing <69516975+ziqing26@users.noreply.github.com> * fix resetAccountAction (#12934) Co-authored-by: Zhang Ziqing <69516975+ziqing26@users.noreply.github.com> * [#12048] Migrate Feedback Rank Option E2E test (#12902) * Initial commit * Fix lint * Follow convention and add test * Change file path * Fix requested changes * Fixed testcases * Fix lint * Add deepcopy * Fixed e2e test --------- Co-authored-by: Wei Qing <48304907+weiquu@users.noreply.github.com> Co-authored-by: Cedric Ong <67156011+cedricongjh@users.noreply.github.com> * [#12048] Migrate FeedbackMcqQuestionE2ETest (#12820) * Migrate MCQ E2E * Fix lint * Fix lint * Update xml --------- Co-authored-by: Cedric Ong <67156011+cedricongjh@users.noreply.github.com> * [#12048] Remove unnecessary loading of datastore entities in InstructorNotificationsPageE2ETest (#12911) * migrate instructor notif e2e --------- Co-authored-by: Cedric Ong <67156011+cedricongjh@users.noreply.github.com> * [#12048] Migrate InstructorCourseDetailsPageE2ETest (#12908) * Add teammates.e2e.cases.sql.InstructorCourseDetailsPageE2ETest * Remove data properly to prevent clashes * Add SQL data bundle * Verify loaded details * Use email address when getting a student row * Check student links * Verify the sending of invites * Verify the reminding of all students to join * Remove SQL data properly to prevent clashes * Verify the downloading of the student list * Implement helper methods for Student * Add BaseTestCaseWithSqlDatabaseAccess::verifyAbsentInDatabase * Add to testng-e2e-sql.xml * Verify the deleting of students * Verify the deleting of all the students * Fix lint * Remove duplicate equality check for students * [#12588] add unit tests for question submission form (#12897) Co-authored-by: Zhang Ziqing <69516975+ziqing26@users.noreply.github.com> * Update developers.json (#12958) * Merge pull request #12960 from TEAMMATES/master (#12961) * [#12048] Fix account request indexing (#12967) * Add isTransactionNeeded method to Action * Remove delay from taskqueuer * Change CreateAccountRequest to handle own transactions * configure agroal connection pool (#12971) * [#12048] Configure connection pool using hikari (#12978) * Configure hikari * Remove spacing * Lint * [#12048] Update liquibase configuration (#12930) * Update gradle config * Update liquibase config for v9 * Turn off table generate for prod * Update of changelog file * Add configuration for generating changelog * Add schema migration docs --------- Co-authored-by: FergusMok <FergusMok1@gmail.com> * [#12048] Migrate AccountRequestsLogicTest (#12780) * Migrate test cases for AccountRequestsLogic * Remove test case * Split test cases * [#12048] Migrate AdminSearchPageE2ETest SQL (#12811) * test e2e changes * fix: reduce e2e test json file size * fix student key * fix course key * fix instructor keys * fix filepath * fix e2e test * remove extra data from bundle * Add correct removal logic to avoid constraint violation * Fix e2e tests and lint fix reset google id test fix e2e tests fix e2e tests fix tests remove double click fix unknown symbol add toast check change toast verification message remove toast check * fix: add null check * move admin search page e2e test to sql cases * Rename AdminSearchPageE2ETest_SQLEntities.json to AdminSearchPageE2ETest_SqlEntities.json * fix failing test * fix: remove extra null check * fix: add test to e2e sql xml file * fix function call * remove unnecessary changes * create new file for sql entities * revert unnecessary changes * remove trailing whitespace * add teardown for account requests --------- Co-authored-by: Cedric Ong <67156011+cedricongjh@users.noreply.github.com> * [#12995] Create documentation for unit tests (#12996) * Create documentation for unit tests * Update docs/unit-testing.md Co-authored-by: Zhang Ziqing <69516975+ziqing26@users.noreply.github.com> * Update docs/unit-testing.md Co-authored-by: Zhang Ziqing <69516975+ziqing26@users.noreply.github.com> --------- Co-authored-by: Zhang Ziqing <69516975+ziqing26@users.noreply.github.com> * [#12048] Remove feedbackSession attributes @fetch annotation (#12992) * Remove feedbackSession @fetch annotation * [#12048] create skeleton for sql LNP tests (#12994) * create skelton for sql LNP tests * allow lnp test to access sql storage and ensure sql lnp tests are independant of each other --------- Co-authored-by: Zhang Ziqing <69516975+ziqing26@users.noreply.github.com> * [#12048] Migrate FeedbackNumScaleQuestionE2ETest (#12940) * Migrate num scale e2e * Fix team id * Fix bugs * Add v9.0.0 tag to liquibase changelog (#13005) * sort courses by id before comparison (#13003) Co-authored-by: Dominic Lim <46486515+domlimm@users.noreply.github.com> --------- Co-authored-by: Nada Ayesh <nayesh10@students.iugaza.edu.ps> Co-authored-by: FergusMok <FergusMok1@gmail.com> Co-authored-by: Maureen Chang <76696006+techMedMau@users.noreply.github.com> Co-authored-by: Cedric Ong <67156011+cedricongjh@users.noreply.github.com> Co-authored-by: Dominic Lim <46486515+domlimm@users.noreply.github.com> Co-authored-by: Nicolas <25302138+NicolasCwy@users.noreply.github.com> Co-authored-by: Ching Ming Yuan <cmingyuan123@gmail.com> Co-authored-by: Wei Qing <48304907+weiquu@users.noreply.github.com> Co-authored-by: DS <yeodisheng@gmail.com> Co-authored-by: Jay Aljelo Ting <65202977+jayasting98@users.noreply.github.com> Co-authored-by: Xenos F <git@xenosfio.com> Co-authored-by: domoberzin <74132255+domoberzin@users.noreply.github.com> Co-authored-by: Marques Tye Jia Jun <97437396+marquestye@users.noreply.github.com>
TEAMMATES · Apr 13, 2024 · 2bd06e2 · 2bd06e2
1 parent 451a25a
commit 2bd06e2
Show file tree

Hide file tree

Showing 30 changed files with 2,505 additions and 114 deletions.
diff --git a/build.gradle b/build.gradle
@@ -77,8 +77,7 @@ dependencies {
     implementation("org.jsoup:jsoup:1.15.2")
     implementation("org.hibernate.orm:hibernate-core:6.1.6.Final")
     implementation("org.postgresql:postgresql:42.7.2")
-    implementation("org.hibernate.orm:hibernate-agroal:6.1.6.Final")
-    implementation("io.agroal:agroal-pool:2.1")
+    implementation("org.hibernate:hibernate-hikaricp:6.1.6.Final")
 
     testAnnotationProcessor(testng)
 
@@ -102,7 +101,9 @@ dependencies {
         exclude group: "org.apache.jmeter", module: "bom"
     }
 
+    liquibaseRuntime("org.liquibase:liquibase-core:4.19.0")
     liquibaseRuntime("info.picocli:picocli:4.7.1")
+    liquibaseRuntime("org.postgresql:postgresql:42.7.2")
     liquibaseRuntime(sourceSets.main.output)
 }
 
@@ -137,6 +138,10 @@ sourceSets {
     }
 }
 
+if (!project.hasProperty("runList")) {
+    project.ext.set("runList", "main")
+}
+
 liquibase {
     activities {
         main {
@@ -146,7 +151,23 @@ liquibase {
             username project.properties['liquibaseUsername']
             password project.properties['liquibasePassword']
         }
+        snapshot {
+            url project.properties['liquibaseDbUrl']
+            username project.properties['liquibaseUsername']
+            password project.properties['liquibasePassword']
+            snapshotFormat "json"
+            outputFile "liquibase-snapshot.json"
+        }
+        diffMain {
+            searchPath "${projectDir}"
+            changeLogFile "src/main/resources/db/changelog/db.changelog-new.xml"
+            referenceUrl  project.properties['liquibaseDbUrl']
+            referenceUsername project.properties['liquibaseUsername']
+            referencePassword project.properties['liquibasePassword']
+            url "offline:postgresql?snapshot=liquibase-snapshot.json"
+        }
     }
+    runList = project.ext.runList
 }
 
 tasks.withType(cz.habarta.typescript.generator.gradle.GenerateTask) {

diff --git a/docs/_markbind/layouts/default.md b/docs/_markbind/layouts/default.md
@@ -27,6 +27,7 @@
   * [Captcha]({{ baseUrl }}/captcha.html)
   * [Documentation]({{ baseUrl }}/documentation.html)
   * [Emails]({{ baseUrl }}/emails.html)
+  * [Unit Testing]({{ baseUrl }}/unit-testing.html)
   * [End-to-End Testing]({{ baseUrl }}/e2e-testing.html)
   * [Performance Testing]({{ baseUrl }}/performance-testing.html)
   * [Accessibility Testing]({{ baseUrl }}/axe-testing.html)

diff --git a/docs/development.md b/docs/development.md
@@ -291,7 +291,9 @@ There are two big categories of testing in TEAMMATES:
 - **Component tests**: white-box unit and integration tests, i.e. they test the application components with full knowledge of the components' internal workings. This is configured in `src/test/resources/testng-component.xml` (back-end) and `src/web/jest.config.js` (front-end).
 - **E2E (end-to-end) tests**: black-box tests, i.e. they test the application as a whole without knowing any internal working. This is configured in `src/e2e/resources/testng-e2e.xml`. To learn more about E2E tests, refer to this [document](e2e-testing.md).  
 
-#### Running the tests
+<div id="running-tests">
+
+#### Running tests
 
 ##### Frontend tests
 
@@ -335,6 +337,8 @@ You can generate the coverage data with `jacocoReport` task after running tests,
 
 The report can be found in the `build/reports/jacoco/jacocoReport/` directory.
 
+</div>
+
 ## Deploying to a staging server
 
 > `Staging server` is the server instance you set up on Google App Engine for hosting the app for testing purposes.

diff --git a/docs/schema-migration.md b/docs/schema-migration.md
@@ -0,0 +1,34 @@
+<frontmatter>
+  title: "Schema Migration"
+</frontmatter>
+
+# SQL Schema Migration
+
+Teammates uses _[Liquibase]_(https://docs.liquibase.com/start/home.html), a database schema change management solution that enables developers to revise and release database changes to production. The maintainers in charge of releases (Release Leader) will be in charge of generating a _Liquibase_ changelog prior to each release to keep the production databases schema in sync with the code. Therefore this section is just for documentation purposes for contributors.
+
+## Liquibase in Teammates
+_Liquibase_ is made available using the [gradle plugin](https://github.com/liquibase/liquibase-gradle-plugin), providing _liquibase_ functions as tasks. Try `gradle tasks | grep "liquibase"` to see all the tasks available. In teammates, change logs (more in the next section) are written in _XML_.
+
+### Liquibase connection
+Amend the `liquibaseDbUrl`, `liquibaseUsername` and `liquibasePassword` in `gradle.properties` to allow the _Liquibase_ plugin to connect your database.
+
+## Change logs, change sets and change types
+A _change log_ is a file that contains a series of _change sets_ (analagous to a transaction) which applies _change types_ (actions). You can refer to this page on liquibase on the types of [change types](https://docs.liquibase.com/change-types/home.html) that can be used.
+
+## Gradle Activities for Liquibase
+Activities in Gradle are a way of specifying different variables provided by gradle to the Liquibase plugin. The argument `runList` provided by `-pRunList=<activity_name>` e.g `./gradlew liquibaseSnapshot -PrunList=snapshot` is used to specify which activity to be used for the Liquibase command. In this case the `liquibaseSnapshot` command is run using the `snapshot` activity.
+
+Here is a brief description of the activities defined for Liquibase
+1. Main: The default activity used by Liquibase commands and is used for running changelogs against a database. This is used by default if a `runList` is not defined
+2. Snapshot: Used to specify output format and name for snapshots i.e JSON
+3. diffMain: Specify the reference and the target database to generate changelog that contains operations to update reference database to the state of the target database. i.e the reference is the JSON file generated by the snapshot command, this can be replaced with a live database which is used as reference.
+
+## Generating/ Updating liquibase change logs
+1. Ensure `diff-main` activity in `build.gradle` is pointing to the latest release changelog `src/main/resources/db/changelog/db.changelog-<release_number>.xml`
+2. Delete the `postgres-data` folder to clear any old database schemas
+3. Run `git checkout <reference_branch>` and 
+4. Run the server using `./gradlew serverRun` to generate tables found on branch
+5. Generate snapshot of database by running `./gradlew liquibaseSnapshot -PrunList=snapshot`, the snapshot will be output to `liquibase-snapshot.json`
+6. Checkout your branch and repeat steps 2 and 4 to generate the tables found on your branch
+7. Run `./gradlew liquibaseDiffChangeLog -PrunList=diffMain` to generate changeLog to resolve database schema differences
+
diff --git a/docs/unit-testing.md b/docs/unit-testing.md
@@ -0,0 +1,206 @@
+<frontmatter>
+  title: "Unit Testing"
+</frontmatter>
+
+# Unit Testing  
+
+## What is Unit Testing?
+
+Unit testing is a testing methodology where the objective is to test components in isolation.
+
+- It aims to ensure all components of the application work as expected, assuming its dependencies are working.   
+- This is done in TEAMMATES by using mocks to simulate a component's dependencies.  
+
+Frontend Unit tests in TEAMMATES are located in `.spec.ts` files, while Backend Unit tests in TEAMMATES can be found in the package `teammates.test`.
+
+
+## Writing Unit Tests
+
+### General guidelines
+
+#### Include only relevant details in tests
+When writing unit tests, reduce the amount of noise in the code to make it easier for future developers to follow.
+
+The code below has a lot of noise in creation of the `studentModel`:
+
+```javascript
+it('displayInviteButton: should display "Send Invite" button when a student has not joined the course', () => {
+    component.studentModels = [
+      {
+        student: {
+          name: 'tester',
+          teamName: 'Team 1',
+          email: 'tester@tester.com',
+          joinState: JoinState.NOT_JOINED,
+          sectionName: 'Tutorial Group 1',
+          courseId: 'text-exa.demo',
+        },
+        isAllowedToViewStudentInSection: true,
+        isAllowedToModifyStudent: true,
+      },
+    ];
+
+    expect(sendInviteButton).toBeTruthy();
+});
+```
+
+However, what is important is only the student joinState. We should thus reduce the noise by including only the relevant details:
+
+```javascript
+it('displayInviteButton: should display "Send Invite" button when a student has not joined the course', () => {
+    component.studentModels = [
+      studentModelBuilder
+        .joinState(JoinState.NOT_JOINED)
+        .build()
+    ];
+
+    expect(sendInviteButton).toBeTruthy();
+});
+```
+
+Including only the relevant details in tests makes it easier for future developers to read and understand the purpose of the test.
+
+#### Favor readability over uniqueness
+Since tests don't have tests, it should be easy for developers to manually inspect them for correctness, even at the expense of greater code duplication.
+
+Take the following test for example:
+
+```java
+@BeforeMethod
+public void setUp() {
+    users = new User[]{new User("alice"), new User("bob")};
+}
+
+@Test
+public void test_register_canRegisterMultipleUsers() {
+    registerAllUsers();
+    for (User user : users) {
+        assertTrue(forum.hasRegisteredUser(user));
+    }
+}
+
+private void registerAllUsers() {
+    for (User user : users) {
+        forum.register(user);
+    }
+}
+```
+
+While the code reduces duplication, it is not as straightforward for a developer to follow.
+
+A more readable way to write this test would be:
+```java
+@Test
+public void test_register_canRegisterMultipleUsers() {
+    User user1 = new User("alice");
+    User user2 = new User("bob");
+
+    forum.register(user1);
+    forum.register(user2);
+
+    assertTrue(forum.hasRegisteredUser(user1));
+    assertTrue(forum.hasRegisteredUser(user2));
+}
+```
+
+By choosing readability over uniqueness in writing unit tests, there is code duplication, but the test flow is easier for a reader to follow.
+
+
+#### Inline mocks in test code
+
+Inlining mock return values in the unit test itself improves readability:
+
+```javascript
+it('getStudentCourseJoinStatus: should return true if student has joined the course' , () => {
+    jest.spyOn(courseService, 'getJoinCourseStatus')
+        .mockReturnValue(of({ hasJoined: true }));
+
+    expect(student.getJoinCourseStatus).toBeTruthy();
+});
+```
+
+By injecting the values in the test right before they are used, developers are able to more easily trace the code and understand the test.
+
+### Frontend
+
+#### Naming
+Unit tests for a function should follow the format:
+
+`"<function-name>: should ... when/if ..."`
+
+Example:
+
+```javascript
+  it('hasSection: should return false when there are no sections in the course')
+```
+
+#### Creating test data
+To aid with [including only relevant details in tests](#include-only-relevant-details-in-tests), use the builder in `src/web/test-helpers/generic-builder.ts`
+
+Usage:
+```javascript
+const instructorModelBuilder = createBuilder<InstructorListInfoTableRowModel>({
+    email: 'instructor@gmail.com',
+    name: 'Instructor',
+    hasSubmittedSession: false,
+    isSelected: false,
+});
+
+it('isAllInstructorsSelected: should return false if at least one instructor !isSelected', () => {
+component.instructorListInfoTableRowModels = [
+  instructorModelBuilder.isSelected(true).build(),
+  instructorModelBuilder.isSelected(false).build(),
+  instructorModelBuilder.isSelected(true).build(),
+];
+
+expect(component.isAllInstructorsSelected).toBeFalsy();
+});
+
+```
+
+#### Testing event emission
+In Angular, child components emit events. To test for event emissions, we've provided a utility function in `src/test-helpers/test-event-emitter`
+
+Usage:
+```javascript
+@Output()
+deleteCommentEvent: EventEmitter<number> = new EventEmitter();
+
+triggerDeleteCommentEvent(index: number): void {
+  this.deleteCommentEvent.emit(index);
+}
+
+it('triggerDeleteCommentEvent: should emit the correct index to deleteCommentEvent', () => {
+  let emittedIndex: number | undefined;
+  testEventEmission(component.deleteCommentEvent, (index) => { emittedIndex = index; });
+
+  component.triggerDeleteCommentEvent(5);
+  expect(emittedIndex).toBe(5);
+});
+```
+
+### Backend
+
+#### Naming
+Unit test names should follow the format: `test<functionName>_<scenario>_<outcome>`
+
+Examples:
+```java
+public void testGetComment_commentDoesNotExist_returnsNull()
+public void testCreateComment_commentDoesNotExist_success()
+public void testCreateComment_commentAlreadyExists_throwsEntityAlreadyExistsException()
+```
+
+#### Creating test data
+To aid with [including only relevant details in tests](#include-only-relevant-details-in-tests), use the `getTypicalX` functions in `BaseTestCase`, where X represents an entity.
+
+Example:
+```java
+Account account = getTypicalAccount();
+account.setEmail("newemail@teammates.com");
+
+Student student = getTypicalStudent();
+student.setName("New Student Name");
+```
+
+<include src="development.md#running-tests" />
diff --git a/src/client/java/teammates/client/scripts/testdataconversion/DataStoreToSqlConverter.java b/src/client/java/teammates/client/scripts/testdataconversion/DataStoreToSqlConverter.java
@@ -57,8 +57,7 @@ public class DataStoreToSqlConverter {
     private UuidGenerator accounRequestUuidGenerator = new UuidGenerator(initialAccountRequestNumber, uuidPrefix);
     private UuidGenerator sectionUuidGenerator = new UuidGenerator(initialSectionNumber, uuidPrefix);
     private UuidGenerator teamUuidGenerator = new UuidGenerator(initialTeamNumber, uuidPrefix);
-    private UuidGenerator deadlineExtensionUuidGenerator = new UuidGenerator(initialDeadlineExtensionNumber,
-            uuidPrefix);
+    private UuidGenerator deadlineExtensionUuidGenerator = new UuidGenerator(initialDeadlineExtensionNumber, uuidPrefix);
     private UuidGenerator instructorUuidGenerator = new UuidGenerator(initialInstructorNumber, uuidPrefix);
     private UuidGenerator studentUuidGenerator = new UuidGenerator(initialStudentNumber, uuidPrefix);
     private UuidGenerator feedbackSessionUuidGenerator = new UuidGenerator(intitialFeedbackSessionNumber, uuidPrefix);
@@ -262,11 +261,9 @@ protected Student convert(StudentAttributes student) {
      */
     protected DeadlineExtension convert(DeadlineExtensionAttributes deadlineExtension) {
         FeedbackSession sqlFeedbackSession = feedbackSessions.get(
-                generatefeedbackSessionKey(deadlineExtension.getCourseId(),
-                        deadlineExtension.getFeedbackSessionName()));
+                generatefeedbackSessionKey(deadlineExtension.getCourseId(), deadlineExtension.getFeedbackSessionName()));
 
-        // User is not included since DataBundleLogic.java does not read users from this
-        // attribute
+        // User is not included since DataBundleLogic.java does not read users from this attribute
         DeadlineExtension sqlDE = new DeadlineExtension(null,
                 sqlFeedbackSession,
                 deadlineExtension.getEndTime());

diff --git a/src/e2e/java/teammates/e2e/cases/AdminSearchPageE2ETest.java b/src/e2e/java/teammates/e2e/cases/AdminSearchPageE2ETest.java
@@ -31,7 +31,7 @@ protected void prepareTestData() {
         putDocuments(testData);
         sqlTestData = loadSqlDataBundle("/AdminSearchPageE2ETest_SqlEntities.json");
         removeAndRestoreSqlDataBundle(sqlTestData);
-        doPutDocumentsSql(sqlTestData);
+        putSqlDocuments(sqlTestData);
     }
 
     @Test

diff --git a/src/e2e/java/teammates/e2e/cases/axe/AdminSearchPageAxeTest.java b/src/e2e/java/teammates/e2e/cases/axe/AdminSearchPageAxeTest.java
@@ -25,7 +25,7 @@ protected void prepareTestData() {
         putDocuments(testData);
         sqlTestData = loadSqlDataBundle("/AdminSearchPageE2ETest_SqlEntities.json");
         removeAndRestoreSqlDataBundle(sqlTestData);
-        doPutDocumentsSql(sqlTestData);
+        putSqlDocuments(sqlTestData);
     }
 
     @Test