8000 DACCESS-8181 - Cucumber cleanup by chrisrlc · Pull Request #2314 · cul-it/blacklight-cornell · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

DACCESS-8181 - Cucumber cleanup #2314

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jul 8, 2025
Merged

DACCESS-8181 - Cucumber cleanup #2314

merged 3 commits into from
Jul 8, 2025

Conversation

chrisrlc
Copy link
Contributor
@chrisrlc chrisrlc commented Jun 30, 2025 •
edited
Loading

https://culibrary.atlassian.net/browse/DACCESS-62
https://culibrary.atlassian.net/browse/DISCOVERYACCESS-8181

  • Reduces cucumber test suite run to ~20 minutes on jenkins with default NUM_PROCESSES=2: https://jenkins.library.cornell.edu/view/Blacklight/job/blacklight-cornell-validate-pull-request-mb/job/DACCESS-8181-cuc-usage/
  • Sets log_level in test environment to :warn, instead of :info. This significantly speeds up tests on jenkins, but let me know if folks prefer it when running tests locally, and I can turn this into an environment variable check instead.
  • Removes unused helper method #render_solr_core
  • Removes unused cucumber fixtures
  • Reduces test setup for search-related tests (more can be done, but had to timebox it to fit in the sprint)
  • Removes unused step definitions (more can be done in future sprints: use bundle exec cucumber --format usage to output unused step definitions)
  • Removes references to ENV['COLLECTIONS'] in tests as this doesn't appear to do anything (possibly used in previous iterations of DCP integration?)
  • Fixes cucumber deprecation warnings for --tag option usage

Note 1: I noticed we're getting "undefined steps" warning messages for the I {string} login required step definition used in bookmarks.feature and login_required.feature (unrelated to this PR). Those tests are likely not getting run - we'll need to look into that in a future sprint.
Note 2: Not part of this PR, but related to ticket: configured Jenkins settings to properly show ansi colors, so build logs are more readable, e.g. https://jenkins.library.cornell.edu/view/Blacklight/job/blacklight-cornell-validate-pull-request-mb/job/DACCESS-8181-cuc-usage/

mhk33
mhk33 approved these changes Jun 30, 2025
View reviewed changes
Copy link
Contributor
@mhk33 mhk33 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

Baroquem
Baroquem approved these changes Jul 1, 2025
View reviewed changes
Copy link
Contributor
@Baroquem Baroquem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@chrisrlc chrisrlc force-pushed the DACCESS-8181-cuc-usage branch from 89876e4 to d3df7f1 Compare July 7, 2025 13:50
@chrisrlc chrisrlc force-pushed the DACCESS-8181-cuc-usage branch from 09e1baa to 42bdae1 Compare July 7, 2025 19:51
sarah-cul
sarah-cul approved these changes Jul 7, 2025
View reviewed changes
Copy link
Contributor
@sarah-cul sarah-cul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is awesome

912F
@chrisrlc chrisrlc merged commit e0c3836 into dev Jul 8, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Baroquem Baroquem Baroquem approved these changes

@mhk33 mhk33 mhk33 approved these changes

@sarah-cul sarah-cul sarah-cul approved these changes

@ebtoner ebtoner Awaiting requested review from ebtoner

@JeremyDuncan JeremyDuncan Awaiting requested review from JeremyDuncan

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@chrisrlc @Baroquem @mhk33 @sarah-cul
0