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 issues with ruby-sample integration test #479

Closed
edmorley opened this issue Jul 21, 2022 · 3 comments · Fixed by #757
Closed

Fix issues with ruby-sample integration test #479

edmorley opened this issue Jul 21, 2022 · 3 comments · Fixed by #757
Labels

Comments

@edmorley
Copy link
Member

edmorley commented Jul 21, 2022

The test has several issues:

  1. The test uses call_test_fixture_service rather than something more realistic like ureq
  2. The web server isn't compliant with the HTTP spec (so attempts to use ureq to make a request to it fail)
  3. The app is unnecessarily complicated - it does not need to accept a posted value and invert it - replying with a fixed string to a GET would do (eg just using a Ruby stdlib HTTP server)
  4. It uses a hardcoded sleep, rather than something like:
    // Retries needed since the server takes a moment to start up.
    let mut attempts_remaining = 5;
    let response = loop {
    let response = ureq::get(&url).call();
    if response.is_ok() || attempts_remaining == 0 {
    break response;
    }
    attempts_remaining -= 1;
    thread::sleep(Duration::from_secs(1));
    }
    .unwrap();
  5. It uses Ruby 2.7, so has to use the heroku/buildpacks:20 builder image (rather than the heroku/builder:22 used by other tests), which slows down CI as it means another builder image has to be pulled.
@schneems
Copy link
Contributor

The web server isn't compliant with the HTTP spec (so attempts to use ureq to make a request to it fail)

Can you give me some more context here? What fails?

@edmorley
Copy link
Member Author

edmorley commented Oct 18, 2022

@schneems The test app is not a proper HTTP server, since it uses TCPServer and does not implement the HTTP protocol itself. It echos back the received payload reversed only, and is missing the status line, headers etc:
https://github.com/heroku/libcnb.rs/blob/main/examples/ruby-sample/test-fixtures/simple-ruby-app/app.rb

@edmorley
Copy link
Member Author

edmorley commented Jan 3, 2024

Wontfix since the Ruby example has been removed in #757.

@edmorley edmorley closed this as not planned Won't fix, can't repro, duplicate, stale Jan 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants