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

Add routingModes option to db profile #295

Closed

Conversation

lamBOOO
Copy link
Contributor

@lamBOOO lamBOOO commented Jul 18, 2023

Sorry for the delay, but I started working on #287. It seems to work so far.

I'm a little confused with the testing, so I "only" added an e2e to test all different routingMode options for errors.

Is the testing done properly?

@derhuerst derhuerst closed this in 5ce0129 Jul 25, 2023
@derhuerst
Copy link
Member

Thanks!

I have squashed your commits into one, and adapted that one commit slightly:

  • moved lib/db-routing-modes.js to p/db/routing-modes (if it's DB-specific, let's put it in p/db; if it's not, let's rename it and put it in lib)
  • added a integration test fixture (which is just a set of recorded E2E test HTTP requests)
  • minor code formatting

@lamBOOO
Copy link
Contributor Author

lamBOOO commented Jul 25, 2023

Thank you. 👍

Just for understanding: The integration test fixture is generated by "simply" running the test-integration:record script and then pushing the updated recording.har into the repo? 🤔

@derhuerst
Copy link
Member

The integration test fixture is generated by "simply" running the test-integration:record script and then pushing the updated recording.har into the repo? 🤔

Yes, although – as you have pointed out – running test-integration:record will fail in practice because most test suites/files' T_MOCK are in the past, so querying new HTTP responses from HAFAS won't work.

I have documented some of the gotchas of the testing setup in 19cdde0. If you have more questions (or even want to contribute more guidance), don't hesitate to create a new Issue!

@lamBOOO
Copy link
Contributor Author

lamBOOO commented Jul 26, 2023

Thanks, it's now more clear to me. My initial starting point for testing/contribution was the contributing.md.

great work btw 👍

derhuerst added a commit that referenced this pull request Feb 28, 2024
back-port of 5ce0129 to 5.x

related: #295
related: #287

Co-Authored-By: Jannis R <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants