Fix part of #17712 Add more acceptance tests for logged in users #20126

agallop · 2024-04-03T10:59:21Z

Overview

This PR fixes or fixes part of Acceptance Testing - covering all user journeys in form of user stories #17712
This PR does the following: Adds several acceptance tests for logged-in users:
- click-all-buttons-on-teach-page
- click-all-links-on-creator-guidelines-page
- click-all-links-on-privacy-policy-page
- click-all-social-icons-in-oppia-footer
- click-all-links-on-terms-page
- click-more-links-in-the-oppia-footer
(For bug-fixing PRs only) The original bug occurred because: N/A

Essential Checklist

Please follow the instructions for making a code change.

I have linked the issue that this PR fixes in the "Development" section of the sidebar.
I have checked the "Files Changed" tab and confirmed that the changes are what I want to make.
I have written tests for my code.
The PR title starts with "Fix #bugnum: " or "Fix part of #bugnum: ...", followed by a short, clear summary of the changes.
I have assigned the correct reviewers to this PR (or will leave a comment with the phrase "@{{reviewer_username}} PTAL" if I can't assign them directly).

Proof that changes are correct

Here are the github action runs for the acceptance tests for this PR which shows that:

The github actions are configured correctly.
The tests run to completion and do not fail for desktop and mobile.
No existing tests have started to fail.

Action summary

Proof of changes on desktop with slow/throttled network

N/A

Proof of changes on mobile phone

Action summary

Proof of changes in Arabic language

N/A

PR Pointers

Never force push! If you do, your PR will be closed.
To reply to reviewers, follow these instructions: https://github.com/oppia/oppia/wiki/Make-a-pull-request#step-5-address-review-comments-until-all-reviewers-approve
Some e2e tests are flaky, and can fail for reasons unrelated to your PR. We are working on fixing this, but in the meantime, if you need to restart the tests, please check the "If your build fails" wiki page.
See the Code Owner's wiki page for what code owners will expect.

…nto static-pages

…l likns on get started page spec

seanlip

Thanks @agallop ! I don't really have comments beyond what Christie has :)

seanlip · 2024-05-10T18:11:18Z

core/tests/puppeteer-acceptance-tests/user-utilities/logged-in-users-utils.ts

+   */
+  async clickLinkToHomePageOnPrivacyPolicyPage(): Promise<void> {
+    await this.page.waitForXPath(
+      '//a[contains(text(),"https://www.oppia.org")]'


Yes, I think so -- the policy is only for the oppia.org site.

seanlip · 2024-05-10T18:11:40Z

core/tests/puppeteer-acceptance-tests/user-utilities/logged-in-users-utils.ts

+      this.clickOn('https://www.oppia.org'),
+    ]);
+
+    expect(this.page.url()).toBe(learnerDashboardUrl);


I think it redirects to the learner dashboard if you are signed in ... are you sure you are signed in?

oppiabot · 2024-05-11T11:50:40Z

Hi @agallop. Due to recent changes in the "develop" branch, this PR now has a merge conflict. Please follow this link if you need help resolving the conflict, so that the PR can be merged. Thanks!

imchristie

I commented on the function clickLinkToHomePageOnPrivacyPolicyPage in logged-in-users-utils.ts

imchristie · 2024-05-13T05:16:01Z

core/tests/puppeteer-acceptance-tests/user-utilities/logged-in-users-utils.ts

+      this.clickOn('https://www.oppia.org'),
+    ]);
+
+    expect(this.page.url()).toBe(learnerDashboardUrl);


Yes, I'm sure I signed in and when I clicked the link. It navigates to 'https://www.oppia.org' instead of the learner dashboard.
The redirecting to the learner dashboard only happens if the link is localhost:8181.

agallop · 2024-05-16T19:27:42Z

@imchristie @seanlip Please take another look

imchristie

LGTM!

oppiabot · 2024-05-20T05:36:40Z

Unassigning @imchristie since they have already approved the PR.

seanlip

Sorry for the late reply -- LGTM for my original comments!

seanlip · 2024-05-25T18:51:13Z

@agallop There's a test failing for the footer -- could you PTAL?

Failures:
1. Logged-in Users should be able to visit the Oppia Twitter
Message:
Expected '
https://x.com/oppiaorg?mx=2' to contain 'twitter.com'.
Stack:
Error: Expected 'https://x.com/oppiaorg?mx=2' to contain 'twitter.com'.
    at <Jasmine>
    at BaseUser.openSocialLinkInNewTabViaIcon (/home/runner/work/oppia/oppia/core/tests/puppeteer-acceptance-tests/user-utilities/logged-in-users-utils.ts:1334:31)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at BaseUser.clickTwitterIconInFooter (/home/runner/work/oppia/oppi

oppiabot · 2024-05-25T18:53:34Z

Hi @agallop, this PR is ready to be merged. Please address any remaining comments prior to merging, and feel free to ask someone to merge your PR once the CI checks pass and you're happy with it. Thanks!

agallop and others added 30 commits March 14, 2024 02:00

Footer navigation logged-in user tests progress

fdc8b10

merge develop into static-pages

2efc822

Click all buttons on get started page done

39a9be3

logged in users get started page test refactoring

22245ed

click all about oppia footer buttons test working

c159ba0

Lint fixes

7a4a117

Lint fixes

514b16e

Lint fixes

ff4c099

null-safety

41876ac

Add /usr/bin/google-chrome-stable as possible CHROME_BIN path

d8d4061

click all about oppia footer buttons changes

ce8c8ef

Merge branch 'develop' into static-pages

9b60f49

Fix some flakiness for logged in user acceptance tests

6c79021

Merge branch 'static-pages' of https://www.github.com/agallop/oppia i…

f3a47bf

…nto static-pages

Remove extra space

f48f90d

Matcher changes for logged-in-users-utils

6e3861a

Swap ` with \' on non-format string

bf6e8d6

Swaping a .toContain to a .toBe

ba26c45

merge develop into static-pages

b6da51b

Rename of some logged in user tests and restructuring of the click al…

10eb873

…l likns on get started page spec

Merge branch 'develop' into static-pages

fd6a7c2

Click all links on creator guidelines page acceptance test

0795425

More logged-in user test progress

aaefea0

Logged in user tests for teach and terms pages

b2807b1

logged-in-user-utils constant changes

8dba1b7

Logged-in users Social icons and privacy page tests

3101382

Fix linter errors

f9f28de

Last logged-in users test

f6458ae

merge develop into static-pages2

87f4788

remove duplicate member from logged-in-user-utils

ddcdfff

agallop added 2 commits May 9, 2024 15:10

Add test for Google Play button

ca4adc4

Fix typo

c42b0bf

seanlip reviewed May 10, 2024

View reviewed changes

seanlip removed their assignment May 10, 2024

oppiabot bot added the PR: don't merge - HAS MERGE CONFLICTS label May 11, 2024

imchristie reviewed May 13, 2024

View reviewed changes

Merge branch 'develop' into static-pages2

a963957

oppiabot bot removed the PR: don't merge - HAS MERGE CONFLICTS label May 14, 2024

agallop added 7 commits May 14, 2024 17:52

Renaming symbols and more test reshuffling

db66722

move some constants to test-contants.ts

ec3e95b

move some constants to test-contants.ts

f9da120

fix incorrect named symbol

732e0c5

Replace tests that where accidentaly removed when merging

20d0bc2

Add test to click Privacy Policy link on terms page

10d842c

More logged in test changes

64745ca

agallop assigned seanlip and imchristie and unassigned agallop May 16, 2024

imchristie approved these changes May 20, 2024

View reviewed changes

oppiabot bot unassigned imchristie May 20, 2024

Merge branch 'develop' into static-pages2

a5ce77d

seanlip approved these changes May 25, 2024

View reviewed changes

seanlip assigned agallop and unassigned seanlip May 25, 2024

oppiabot bot added the PR: LGTM label May 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix part of #17712 Add more acceptance tests for logged in users #20126

Fix part of #17712 Add more acceptance tests for logged in users #20126

agallop commented Apr 3, 2024 •

edited

seanlip left a comment

seanlip May 10, 2024

seanlip May 10, 2024

oppiabot bot commented May 11, 2024

imchristie left a comment

imchristie May 13, 2024

agallop commented May 16, 2024

imchristie left a comment

oppiabot bot commented May 20, 2024

seanlip left a comment

seanlip commented May 25, 2024

oppiabot bot commented May 25, 2024

Fix part of #17712 Add more acceptance tests for logged in users #20126

Are you sure you want to change the base?

Fix part of #17712 Add more acceptance tests for logged in users #20126

Conversation

agallop commented Apr 3, 2024 • edited

Overview

Essential Checklist

Proof that changes are correct

Proof of changes on desktop with slow/throttled network

Proof of changes on mobile phone

Proof of changes in Arabic language

PR Pointers

seanlip left a comment

Choose a reason for hiding this comment

seanlip May 10, 2024

Choose a reason for hiding this comment

seanlip May 10, 2024

Choose a reason for hiding this comment

oppiabot bot commented May 11, 2024

imchristie left a comment

Choose a reason for hiding this comment

imchristie May 13, 2024

Choose a reason for hiding this comment

agallop commented May 16, 2024

imchristie left a comment

Choose a reason for hiding this comment

oppiabot bot commented May 20, 2024

seanlip left a comment

Choose a reason for hiding this comment

seanlip commented May 25, 2024

oppiabot bot commented May 25, 2024

agallop commented Apr 3, 2024 •

edited