Teammates: StudentCourseDetailsPageUiTest: fix failure against dev server

[damithc]

v5.104
Not sure why this fails against my dev server but passes on Travis (also fails against live server)

java.lang.AssertionError:  


    at org.testng.AssertJUnit.assertEquals(AssertJUnit.java:101)
    at org.testng.AssertJUnit.assertEquals(AssertJUnit.java:108)
    at teammates.test.driver.HtmlHelper.assertSameHtml(HtmlHelper.java:85)
    at teammates.test.driver.HtmlHelper.assertSameHtml(HtmlHelper.java:53)
    at teammates.test.pageobjects.AppPage.verifyHtmlPart(AppPage.java:850)
    at teammates.test.pageobjects.AppPage.verifyHtml(AppPage.java:824)
    at teammates.test.cases.browsertests.StudentCourseDetailsPageUiTest.verifyContent(StudentCourseDetailsPageUiTest.java:46)
    at teammates.test.cases.browsertests.StudentCourseDetailsPageUiTest.testAll(StudentCourseDetailsPageUiTest.java:27)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:606)
    at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:85)
    at org.testng.internal.Invoker.invokeMethod(Invoker.java:659)
    at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:845)
    at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1153)
    at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:125)
    at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:108)
    at org.testng.TestRunner.privateRun(TestRunner.java:771)
    at org.testng.TestRunner.run(TestRunner.java:621)
    at org.testng.SuiteRunner.runTest(SuiteRunner.java:357)
    at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:352)
    at org.testng.SuiteRunner.privateRun(SuiteRunner.java:310)
    at org.testng.SuiteRunner.run(SuiteRunner.java:259)
    at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
    at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:86)
    at org.testng.TestNG.runSuitesSequentially(TestNG.java:1199)
    at org.testng.TestNG.runSuitesLocally(TestNG.java:1124)
    at org.testng.TestNG.run(TestNG.java:1032)
    at org.testng.IDEARemoteTestNG.run(IDEARemoteTestNG.java:72)
    at org.testng.RemoteTestNGStarter.main(RemoteTestNGStarter.java:127)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:606)
    at com.intellij.rt.execution.application.AppMain.main(AppMain.java:147)

[amarlearning]

@damithc can you post the screenshot of the failure exception.
Eclipse > Result of running suite > Failed test

[damithc]

@damithc can you post the screenshot of the failure exception.

@amarlearning The stack trace is given. Also, the difference between the actual and expected is given (see screenshot). I didn't post the full screenshot because those are the two most important things. Any missing info you are looking for?

[VamsiSangam]

The cause of that is my recent merge again :sweat: . Sorry for causing so many issues. Will be careful from next time.

[damithc]

The cause of that is my recent merge again

@thenaesh how is this not detected by travis?

[amarlearning]

@VamsiSangam same happened with me, ran godmode on failing test and got this change. But @damithc sir in my case the Travis CI build is failing.

[damithc]

Not entirely your fault :-) This real culprit is the fact that we have tests that don't always pass and needs retrying. Ideally, all tests should pass every time. That's something we need to address in the longer term. In any case, has this specific case already rectified in a later commit? @VamsiSangam

[amarlearning]

@damithc no, it's not rectified link

[VamsiSangam]

No, it is not rectified sir. Can you post all the failing cases sir, so that I can submit a PR to rectify it?

[amarlearning]

@VamsiSangam I will rectify that with this PR

[damithc]

I will rectify that with this PR

@amarlearning This problem should be rectified in a separate PR (a PR should solve no more than one problem).

@VamsiSangam to find other failing test cases, add a MOTD in build.properties (e.g. app.student.motd.url = test-student-motd.html) and run the test suite.

[wkurniawan07]

@VamsiSangam actually your recent merge fixes it; it broke at the same time you removed the MOTD container.
How Travis allows both variants is still beyond me for now.

[whipermr5]

@VamsiSangam @amarlearning The link actually shows that it has been rectified. As highlighted by @wkurniawan07, the default picture URL is what should be in the file - not the full URL.

I went digging around to understand the mechanism, and found out that the full URL:

/page/studentProfilePic?studentemail=${student.email.enc}&courseid=${course.id.enc}

is always included in the initial studentCourseDetailsPage HTML response sent to the browser. When the browser accesses this URL, it receives a response with HTTP status 1 No image found (weird custom status; looks like a bug in StudentProfilePictureAction). Subsequently, the image is shown to be broken in the browser. A few moments later, when the document ready event fires, this JavaScript function gets called, replacing the URL with the default picture URL:

/images/profile_picture_default.png

The replacement from full URL to default picture URL sometimes fails unpredictably when, for whatever reason, the JavaScript doesn't fire properly. If this failure happens when GodMode is on, the test HTML file gets incorrectly updated to the full URL instead of the default picture URL.

The solution would be to fix this unreliable frontend mechanism. Instead, the backend (StudentProfilePictureAction) should send an HTTP 302 that redirects the browser to /images/profile_picture_default.png. This way, the HTML would always show the full URL, while the user correctly sees the default profile picture (instead of an ugly broken image which is currently shown whenever the JavaScript fails to fire).

[whipermr5]

@damithc This issue has been fixed in c42cb9f. I've opened #7365 to report the underlying issue.

[damithc]

@damithc This issue has been fixed in c42cb9f.

Great. closing.