[#9536] [#9480] Add front-end, snapshot, E2E tests for admin home page #9605
Conversation
wkurniawan07
force-pushed
the
admin-home-tests
branch
2 times, most recently
from
a264713
to
d3532f3
Compare
wkurniawan07
force-pushed
the
admin-home-tests
branch
from
d3532f3
to
21b3a52
Compare
| */ | ||
| public class AdminHomePage extends AppPage { | ||
|
|
||
| @FindBy(id = "instructor-details-single-line") |
There was a problem hiding this comment.
Is it a good practice to actually add an id for testing only? Cos I have been trying to avoid adding an id directly in my PRs for simple fields
There was a problem hiding this comment.
Hard to say. Adding production code for testing purpose is most of the time frowned upon and this should not be an exception, but this simplifies stuffs by quite a lot.
| @@ -62,8 +55,7 @@ public void testAll() { | |||
| StudentProfileAttributes.Gender.FEMALE, "this is enough!$%&*</>"); | |||
| profilePage.verifyPhotoSize("295px", "295px"); | |||
|
|
|||
| StudentProfileAttributes studentProfileAttributes = | |||
| BackDoor.getStudentProfile(TestProperties.TEST_STUDENT2_ACCOUNT); | |||
| StudentProfileAttributes studentProfileAttributes = BackDoor.getStudentProfile("SProfUiT.student"); | |||
There was a problem hiding this comment.
should we assertNotNull() for the studentProfileAttributes before assertTrue the value? Else the return null may not be useful right?
There was a problem hiding this comment.
Good to have but not necessary. Without assertNotNull the block will hit NPE instead of assertion error, which pretty much is the same thing.
| AdminHomePage homePage = loginAdminToPage(url, AdminHomePage.class); | ||
|
|
||
| String name = "AHPUiT Instrúctör WithPlusInEmail"; | ||
| String email = "AHPUiT+++_.instr1!@gmail.tmt"; |
There was a problem hiding this comment.
whats the naming convention for the +++?
There was a problem hiding this comment.
Why do you need a naming convention? This is just a sample test data.
| @@ -33,9 +26,9 @@ public void testAll() { | |||
|
|
|||
| ______TS("Typical case: Log in with filled profile values"); | |||
There was a problem hiding this comment.
Should we keep this style of describing test cases? On a side note, do you think it is necessary to add a description to testAll() somewhere since we are removing descriptions for all testAll() functions?
src/web/app/pages-admin/admin-home-page/admin-home-page.component.spec.ts
Show resolved
Hide resolved
src/web/app/pages-admin/admin-home-page/admin-home-page.component.spec.ts
Show resolved
Hide resolved
wkurniawan07
added
s.ToMerge
wkurniawan07
added this to Ongoing
in Front-end and RESTful back-end Migration
via automation
wkurniawan07
moved this from Ongoing
to Done
in Front-end and RESTful back-end Migration
Part of #9480 and part of #9536