Fix #20139: Surface Acceptance Test results in a more convenient location #20171

rahat2134 · 2024-04-16T11:37:35Z

Overview

This PR fixes or fixes part of [Feature Request]: Surface Acceptance Test results in a more convenient location #20139
This PR does the following: When looking at the results of acceptance tests in the GitHub workflows, perhaps the most important part of the log, the test results, are buried under logs for parent and supporting processes. (i.e. the initialization and shutdown of the server.).This PR will fix that.

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

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.

oppiabot · 2024-04-16T11:37:37Z

Hi @rahat2134, can you complete the following:

The body of this PR is missing the proof that changes are correct section, please update it to include the section.
Thanks!

oppiabot · 2024-04-16T11:37:47Z

Assigning @StephenYu2018 for the first pass review of this PR. Thanks!

rahat2134 · 2024-04-16T11:39:33Z

@agallop PTAL.

agallop · 2024-04-16T19:08:51Z

Hello Rahat,

The goal of the original issue is to make it so there is a step in the github action that only prints the test output.

For example, I would like this action to only output.

Running suite with 6 specs.

--------------------------------------------------
| 1. Suite started: Logged-in User in About page |
--------------------------------------------------

Spec started : Logged-in User in About page should open Math Classroom page with the Browse Our Lessons button.
-> Passed [ Took 3.04 seconds ]

Spec started : Logged-in User in About page should open Android page with the Access Android App button.
-> Passed [ Took 2.994 seconds ]

Spec started : Logged-in User in About page should open Math Classroom page with the Visit Classroom button.
-> Passed [ Took 3.028 seconds ]

Spec started : Logged-in User in About page should open Community Library page with the Browse Library button.
-> Passed [ Took 3.185 seconds ]

Spec started : Logged-in User in About page should open Creator Dashboard page and Exploration Editor with the Create Lessons button
LOG: The Create Lessons button opens the Creator Dashboard in Create Mode.
LOG: The Create Lessons button displays the Exploration Editor page.
-> Passed [ Took 7.009 seconds ]

Spec started : Logged-in User in About page should open Math Classroom page with the Explore Lessons button.
-> Passed [ Took 5.337 seconds ]



6 specs, 0 failures
Finished in 40.275 seconds

The full output should still print in a separate expansion panel and be able to be printed when running locally.

StephenYu2018

A couple of concerns.

scripts/run_acceptance_tests_test.py

StephenYu2018 · 2024-04-16T23:46:03Z

scripts/run_acceptance_tests_test.py

@@ -349,3 +350,17 @@ def test_start_tests_for_long_lived_process(self) -> None:
        with self.swap_mock_set_constants_to_default:
            with self.swap(constants, 'EMULATOR_MODE', True):
                run_acceptance_tests.main(args=['--suite', 'testSuite'])
+
+    def test_print_test_output(self) -> None:


Please include the result of printing test output in the test name

StephenYu2018 · 2024-04-16T23:46:27Z

scripts/run_acceptance_tests.py

@@ -1,4 +1,4 @@
-# Copyright 2023 The Oppia Authors. All Rights Reserved.
+# Copyright 2024 The Oppia Authors. All Rights Reserved.


Thanks for this! :)

scripts/run_acceptance_tests_test.py

…nce-Test_Result

…/oppia into Acceptance-Test_Result

rahat2134 · 2024-04-29T10:40:58Z

correcting the failing backend tests.

…nce-Test_Result

…/oppia into Acceptance-Test_Result

…nce-Test_Result

…/oppia into Acceptance-Test_Result

rahat2134 · 2024-04-30T16:46:04Z

@agallop PTAL.

agallop · 2024-05-04T15:37:06Z

.github/workflows/e2e_lighthouse_performance_acceptance_tests.yml

@@ -392,8 +392,12 @@ jobs:
          run: python -m scripts.build --prod_env
        - name: Run Desktop Acceptance Test ${{ matrix.suite }}
          run: xvfb-run -a --server-args="-screen 0, 1285x1000x24" python -m scripts.run_acceptance_tests --skip-build --suite=${{ matrix.suite }} --prod_env


Can you add a flag for this new behavior and set it here?

agallop · 2024-05-04T15:46:09Z

scripts/run_acceptance_tests.py

@@ -177,6 +194,9 @@ def main(args: Optional[List[str]] = None) -> None:
    with servers.managed_portserver():
        _, return_code = run_tests(parsed_args)

+    with open('test_output.log', 'r', encoding='utf-8') as output_file:


The test output is being printed a second time here and that's not necessary.

agallop

Thanks Rahat! Some more comments

agallop · 2024-05-04T16:27:18Z

scripts/run_acceptance_tests.py

@@ -99,6 +99,21 @@ def compile_test_ts_files() -> None:
        os.path.join(build_dir_path, 'images'))


+def print_test_output(output_lines: List[bytes]) -> None:


I do think this is all very complex. You can just print all the lines.

In the event of a failure, I don't think this will print the call stack as well. Which we would want to see as it tells you where and why the test failed.

agallop · 2024-05-04T16:28:59Z

.github/workflows/e2e_lighthouse_performance_acceptance_tests.yml

@@ -392,8 +392,12 @@ jobs:
          run: python -m scripts.build --prod_env
        - name: Run Desktop Acceptance Test ${{ matrix.suite }}
          run: xvfb-run -a --server-args="-screen 0, 1285x1000x24" python -m scripts.run_acceptance_tests --skip-build --suite=${{ matrix.suite }} --prod_env
+        - name: Display Desktop Test Output


Display Desktop Acceptance Test Output

agallop · 2024-05-04T16:29:15Z

.github/workflows/e2e_lighthouse_performance_acceptance_tests.yml

        - name: Run Mobile Acceptance Test ${{ matrix.suite }}
          run: xvfb-run -a --server-args="-screen 0, 1285x1000x24" python -m scripts.run_acceptance_tests --skip-build --suite=${{ matrix.suite }} --prod_env --mobile
+        - name: Display Mobile Test Output


Display Mobile Acceptance Test Output

oppiabot · 2024-05-04T16:35:08Z

Unassigning @agallop since the review is done.

oppiabot · 2024-05-04T16:35:11Z

Hi @rahat2134, it looks like some changes were requested on this pull request by @agallop. PTAL. Thanks!

oppiabot · 2024-05-11T17:33:01Z

Hi @rahat2134, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 4 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 4 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

oppiabot · 2024-05-18T18:30:42Z

Hi @rahat2134, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 4 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 4 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

surface the result of acceptance tests

bf7bbb8

rahat2134 requested a review from a team as a code owner April 16, 2024 11:37

rahat2134 requested a review from StephenYu2018 April 16, 2024 11:37

oppiabot bot assigned rahat2134 Apr 16, 2024

oppiabot bot assigned StephenYu2018 Apr 16, 2024

rahat2134 requested review from agallop and removed request for StephenYu2018 April 16, 2024 11:37

rahat2134 assigned agallop and unassigned StephenYu2018 and rahat2134 Apr 16, 2024

Merge branch 'develop' into Acceptance-Test_Result

e602e83

agallop assigned rahat2134 and unassigned agallop Apr 16, 2024

Merge branch 'develop' into Acceptance-Test_Result

4af3798

StephenYu2018 requested changes Apr 17, 2024

View reviewed changes

rahat2134 and others added 5 commits April 18, 2024 09:58

Merge branch 'develop' into Acceptance-Test_Result

901b018

changes requested

0ba363b

Merge branch 'develop' of https://github.com/oppia/oppia into Accepta…

987a4ea

…nce-Test_Result

changes

f5455aa

Merge branch 'Acceptance-Test_Result' of https://github.com/rahat2134…

92c1c1a

…/oppia into Acceptance-Test_Result

rahat2134 requested a review from a team as a code owner April 21, 2024 06:28

rahat2134 requested review from U8NWXD and removed request for U8NWXD April 21, 2024 06:28

rahat2134 added 3 commits April 21, 2024 13:51

changes

cf107ca

changes

569c338

changes

7e03d23

rahat2134 marked this pull request as draft April 29, 2024 10:41

rahat2134 and others added 8 commits April 29, 2024 22:28

Merge branch 'develop' of https://github.com/oppia/oppia into Accepta…

80108cb

…nce-Test_Result

changes

caec6d1

Merge branch 'Acceptance-Test_Result' of https://github.com/rahat2134…

14540d7

…/oppia into Acceptance-Test_Result

Merge branch 'develop' into Acceptance-Test_Result

18a9a48

Merge branch 'develop' into Acceptance-Test_Result

b7a5149

change

138e167

Merge branch 'develop' of https://github.com/oppia/oppia into Accepta…

50a1339

…nce-Test_Result

Merge branch 'Acceptance-Test_Result' of https://github.com/rahat2134…

fd0e56b

…/oppia into Acceptance-Test_Result

rahat2134 marked this pull request as ready for review April 30, 2024 16:45

rahat2134 assigned agallop and unassigned rahat2134 Apr 30, 2024

Merge branch 'develop' into Acceptance-Test_Result

36071fa

agallop reviewed May 4, 2024

View reviewed changes

agallop suggested changes May 4, 2024

View reviewed changes

oppiabot bot unassigned agallop May 4, 2024

oppiabot bot assigned rahat2134 May 4, 2024

oppiabot bot added the stale label May 11, 2024

Merge branch 'develop' into Acceptance-Test_Result

66bbd22

oppiabot bot removed the stale label May 11, 2024

oppiabot bot added the stale label May 18, 2024

Merge branch 'develop' into Acceptance-Test_Result

2683b98

oppiabot bot removed the stale label May 19, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #20139: Surface Acceptance Test results in a more convenient location #20171

Fix #20139: Surface Acceptance Test results in a more convenient location #20171

rahat2134 commented Apr 16, 2024 •

edited

oppiabot bot commented Apr 16, 2024

oppiabot bot commented Apr 16, 2024

rahat2134 commented Apr 16, 2024

agallop commented Apr 16, 2024

StephenYu2018 left a comment

StephenYu2018 Apr 16, 2024

StephenYu2018 Apr 16, 2024

rahat2134 commented Apr 29, 2024

rahat2134 commented Apr 30, 2024

agallop May 4, 2024

agallop May 4, 2024

agallop left a comment

agallop May 4, 2024

agallop May 4, 2024

agallop May 4, 2024

oppiabot bot commented May 4, 2024

oppiabot bot commented May 4, 2024

oppiabot bot commented May 11, 2024

oppiabot bot commented May 18, 2024

		@@ -1,4 +1,4 @@
		# Copyright 2023 The Oppia Authors. All Rights Reserved.
		# Copyright 2024 The Oppia Authors. All Rights Reserved.

		@@ -99,6 +99,21 @@ def compile_test_ts_files() -> None:
		os.path.join(build_dir_path, 'images'))


		def print_test_output(output_lines: List[bytes]) -> None:

Fix #20139: Surface Acceptance Test results in a more convenient location #20171

Are you sure you want to change the base?

Fix #20139: Surface Acceptance Test results in a more convenient location #20171

Conversation

rahat2134 commented Apr 16, 2024 • edited

Overview

Essential Checklist

Proof that changes are correct

PR Pointers

oppiabot bot commented Apr 16, 2024

oppiabot bot commented Apr 16, 2024

rahat2134 commented Apr 16, 2024

agallop commented Apr 16, 2024

StephenYu2018 left a comment

Choose a reason for hiding this comment

StephenYu2018 Apr 16, 2024

Choose a reason for hiding this comment

StephenYu2018 Apr 16, 2024

Choose a reason for hiding this comment

rahat2134 commented Apr 29, 2024

rahat2134 commented Apr 30, 2024

agallop May 4, 2024

Choose a reason for hiding this comment

agallop May 4, 2024

Choose a reason for hiding this comment

agallop left a comment

Choose a reason for hiding this comment

agallop May 4, 2024

Choose a reason for hiding this comment

agallop May 4, 2024

Choose a reason for hiding this comment

agallop May 4, 2024

Choose a reason for hiding this comment

oppiabot bot commented May 4, 2024

oppiabot bot commented May 4, 2024

oppiabot bot commented May 11, 2024

oppiabot bot commented May 18, 2024

rahat2134 commented Apr 16, 2024 •

edited