Skip to content

Increase HTTP timeout in test configuration to fix flaky Cypress mapping tests#1652

Merged
osma merged 1 commit intomainfrom
fix-cypress-concept-mappings-test-http-timeout
Aug 20, 2024
Merged

Increase HTTP timeout in test configuration to fix flaky Cypress mapping tests#1652
osma merged 1 commit intomainfrom
fix-cypress-concept-mappings-test-http-timeout

Conversation

@osma
Copy link
Member

@osma osma commented Aug 20, 2024

Reasons for creating this PR

Some Cypress runs were failing under GitHub Actions CI. In particular the "concept mappings / full" and/or "concept mappings / partial" tests for the concept page often failed. For example this run.

Adjusting the Cypress timeouts doesn't help, because the problem is that the underlying Skosmos resolver code (LinkedDataResolver) times out. The reason is that we've set a 2 second httpTimeout in testconfig.ttl, used to configure Skosmos for the PHPUnit and Cypress tests. This is too short to reliably resolve some of the mappings (LCSH, KOKO, YSA etc.) especially under the GitHub Actions CI environment.

The fix is simple: increase the timeout to 8 seconds. (Sidenote: this needs to be different from the default value of 5 seconds, because it is tested in a PHPUnit test that needs to verify that the non-default setting is effective. The test has been changed accordingly)

Link to relevant issue(s), if any

Description of the changes in this PR

  • increase httpTimeout value from 2 to 8 in testconfig.ttl
  • modify GlobalConfigTest accordingly

Known problems or uncertainties in this PR

Not sure if 8 is the ideal value here but it's better than 2 for sure.

Checklist

  • phpUnit tests pass locally with my changes
  • I have added tests that show that the new code works, or tests are not relevant for this PR (e.g. only HTML/CSS changes)
  • The PR doesn't reduce accessibility of the front-end code (e.g. tab focus, scaling to different resolutions, use of .sr-only class, color contrast)
  • The PR doesn't introduce unintended code changes (e.g. empty lines or useless reindentation)

@osma osma added this to the 3.0 milestone Aug 20, 2024
@osma osma self-assigned this Aug 20, 2024
@sonarqubecloud
Copy link

@osma
Copy link
Member Author

osma commented Aug 20, 2024

Re-running Actions jobs twice, so that the Cypress tests have been run six times under CI in total. If they pass every time, I will merge this.

@codecov
Copy link

codecov bot commented Aug 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.71%. Comparing base (f2cc004) to head (2a391f4).
Report is 27 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1652      +/-   ##
============================================
+ Coverage     70.62%   70.71%   +0.08%     
- Complexity     1645     1647       +2     
============================================
  Files            32       32              
  Lines          4320     4323       +3     
============================================
+ Hits           3051     3057       +6     
+ Misses         1269     1266       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@osma
Copy link
Member Author

osma commented Aug 20, 2024

All tests were successful, merging this.

@osma osma merged commit 58e2cab into main Aug 20, 2024
@osma osma deleted the fix-cypress-concept-mappings-test-http-timeout branch August 20, 2024 08:25
@osma osma modified the milestones: 3.x, 3.0 Jan 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant