Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the e2e test failure in the integration tests #751

Merged
merged 5 commits into from
May 16, 2024
Merged

Conversation

vedhav
Copy link
Contributor

@vedhav vedhav commented May 10, 2024

The root cause of the issue was that the chromote browser was unable to render the PDF file without a plugin with the following error in the terminal:

  ── Error ('test-shinytest2-tm_file_viewer.R:84:3'): e2e - tm_file_viewer: Shows selected url ──
  Error in `app_wait_for_idle(self, private, duration = duration, timeout = timeout)`: An error occurred while waiting for Shiny to be stable
  Backtrace:1. └─app_driver$click(selector = "[id= '6_anchor']") at test-shinytest2-tm_file_viewer.R:84:3
   2.   └─self$wait_for_idle()
   3.     └─shinytest2:::app_wait_for_idle(self, private, duration = duration, timeout = timeout)
   4.       └─shinytest2:::app_abort(self, private, "An error occurred while waiting for Shiny to be stable")
   5.         └─rlang::abort(..., app = self, call = call)

Note that this test is a bit flaky, I was only able to reproduce the error a few times. Sometimes it passed without any error.

To reproduce this issue you can try the following:
Please make sure you have the dev version of teal.

app_driver <- teal:::TealAppDriver$new(
  data = teal.data::teal_data(iris = iris),
  modules = teal.modules.general::tm_file_viewer(
    input_path = list(
      url = "https://fda.gov/files/drugs/published/Portable-Document-Format-Specifications.pdf"
    )
  )
)

app_driver$open_url()
app_driver$view()

The URL in the app_driver$open_url() works fine

Screenshot 2024-05-10 at 3 45 48 PM

The URL in the app_driver$view() shows the error Couldn't load plugin.

Screenshot 2024-05-10 at 3 46 00 PM

The fix is to use some other file format, here we use a PNG file to fix this. In the future, we should test all the files and make sure they work.

@vedhav vedhav added the core label May 10, 2024
Copy link
Contributor

github-actions bot commented May 10, 2024

CLA Assistant Lite bot ✅ All contributors have signed the CLA

@vedhav
Copy link
Contributor Author

vedhav commented May 10, 2024

I have read the CLA Document and I hereby sign the CLA

Copy link
Contributor

github-actions bot commented May 10, 2024

badge

Code Coverage Summary

Filename                      Stmts    Miss  Cover    Missing
--------------------------  -------  ------  -------  ------------------------------------
R/tm_a_pca.R                    827     827  0.00%    108-1068
R/tm_a_regression.R             773     773  0.00%    153-1031
R/tm_data_table.R               185     185  0.00%    93-332
R/tm_file_viewer.R              173     173  0.00%    44-252
R/tm_front_page.R               133     122  8.27%    70-228
R/tm_g_association.R            330     330  0.00%    135-537
R/tm_g_bivariate.R              672     410  38.99%   303-769, 810, 921, 938, 956, 967-989
R/tm_g_distribution.R          1050    1050  0.00%    122-1311
R/tm_g_response.R               351     351  0.00%    154-578
R/tm_g_scatterplot.R            722     722  0.00%    230-1053
R/tm_g_scatterplotmatrix.R      278     259  6.83%    165-472, 533, 547
R/tm_missing_data.R            1069    1069  0.00%    92-1317
R/tm_outliers.R                 985     985  0.00%    134-1263
R/tm_t_crosstable.R             251     251  0.00%    141-440
R/tm_variable_browser.R         830     825  0.60%    79-1071, 1109-1293
R/utils.R                        99      96  3.03%    82-267
R/zzz.R                           2       2  0.00%    2-3
TOTAL                          8730    8430  3.44%

Diff against main

Filename      Stmts    Miss  Cover
----------  -------  ------  --------
TOTAL             0       0  +100.00%

Results for commit: 7c53c47

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@vedhav
Copy link
Contributor Author

vedhav commented May 10, 2024

After replacing the PDF url with a PNG URL the chromote browser was able to render the file without any issue.

The URL in the app_driver$open_url() works fine

Screenshot 2024-05-10 at 3 59 57 PM

The URL in the app_driver$view() also works fine

Screenshot 2024-05-10 at 4 00 21 PM

Copy link
Contributor

github-actions bot commented May 10, 2024

Unit Tests Summary

  1 files   22 suites   10m 18s ⏱️
147 tests 147 ✅ 0 💤 0 ❌
478 runs  478 ✅ 0 💤 0 ❌

Results for commit 7c53c47.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented May 10, 2024

Unit Test Performance Difference

Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
shinytest2-tm_a_pca 💚 $96.12$ $-3.47$ $0$ $0$ $0$ $0$
shinytest2-tm_a_regression 💚 $41.88$ $-2.30$ $0$ $0$ $0$ $0$
shinytest2-tm_data_table 💚 $18.11$ $-1.27$ $0$ $0$ $0$ $0$
shinytest2-tm_file_viewer 💚 $17.71$ $-1.97$ $0$ $0$ $0$ $0$
shinytest2-tm_misssing_data 💚 $33.21$ $-1.06$ $0$ $0$ $0$ $0$
shinytest2-tm_outliers 💚 $84.75$ $-1.74$ $0$ $0$ $0$ $0$
shinytest2-tm_variable_browser 💔 $48.87$ $+1.01$ $0$ $0$ $0$ $0$

Results for commit a6e647b

♻️ This comment has been updated with latest results.

@vedhav vedhav force-pushed the fix-e2e-test@main branch from 66e3fd6 to 0257f9e Compare May 10, 2024 11:15
@m7pr
Copy link
Contributor

m7pr commented May 10, 2024

There is a different R CMD CHeck error that I see in this PR and in my other PR in this repo

@vedhav
Copy link
Contributor Author

vedhav commented May 10, 2024

Yes, that's right @m7pr!
I cannot reproduce this error in my local as well.
There are 8 errors in the CI. 5 in tm_missing_data and 3 in tm_front_page
All of them are during the app driver initialization:

app_driver <- app_driver_tm_front_page()
app_driver <- app_driver_tm_missing_data()

With the error log:

  Error in `app_initialize(self, private, app_dir = app_dir, ..., load_timeout = load_timeout, 
      timeout = timeout, wait = wait, expect_values_screenshot_args = expect_values_screenshot_args, 
      screenshot_args = screenshot_args, check_names = check_names, 
      name = name, variant = variant, view = view, height = height, 
      width = width, seed = seed, clean_logs = clean_logs, shiny_args = shiny_args, 
      render_args = render_args, options = options)`: Shiny app did not become stable in 1e+05ms.
  Message: An error occurred while waiting for Shiny to be stable
  Caused by error in `app_wait_for_idle()`:
  ! An error occurred while waiting for Shiny to be stable

@vedhav vedhav force-pushed the fix-e2e-test@main branch from 90558f4 to 245fcbb Compare May 16, 2024 12:40
@vedhav vedhav enabled auto-merge (squash) May 16, 2024 12:41
@vedhav vedhav merged commit a99ce90 into main May 16, 2024
23 checks passed
@vedhav vedhav deleted the fix-e2e-test@main branch May 16, 2024 13:17
@github-actions github-actions bot locked and limited conversation to collaborators May 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants