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

"adding mine block to test server" #544

Merged
merged 30 commits into from
Sep 22, 2022

Conversation

raphjaph
Copy link
Collaborator

No description provided.

@casey
Copy link
Collaborator

casey commented Sep 20, 2022

Just picked this up and implemented extending the test RPC server blockchain.

I made a Blocks struct so that we avoid having two mutexes, which is a little bit awkward.

The server is polling for updates, so it won't reflect blocks which were just added to the test RPC server. I added thread::sleep(Duration::from_secs(1)), but that slows down tests, so we should come up with a better solution.

One option would be to add a function like this:

impl TestServer {
  fn get(&self, url: &str) -> Response {
    self.sync();
    reqwest::blocking::get(self.join_url(url)).unwrap()
  }

  fn sync(&self) {
    // This is not actually right, I copied this code from existing code in tests/state.rs, but you get the idea
    for attempt in 0.. {
      let best_hash = self.client.get_best_block_hash().unwrap();
      let bitcoind_height = self
        .client
        .get_block_header_info(&best_hash)
        .unwrap()
        .height as u64;

      let ord_height = reqwest::blocking::get(&format!(
        "http://127.0.0.1:{}/height",
        self.ord_http_port.unwrap()
      ))
      .unwrap()
      .text()
      .unwrap()
      .parse::<u64>()
      .unwrap();

      if ord_height == bitcoind_height {
        break;
      } else {
        if attempt == 300 {
          panic!("Ord height {ord_height} did not catch up to bitcoind height {bitcoind_height}");
        }

        sleep(Duration::from_millis(100));
      }
    }
  }
}

@raphjaph
Copy link
Collaborator Author

raphjaph commented Sep 21, 2022

What if we expose the `Index::index() method on the server so that it instantly indexes?

Just some notes:
0. TestServer contains a mock BitcoinRpcServer and an ord::Server

  1. Mock BitcoinRpcServer should run in background
  2. ord::Server should be able to:
    a. run in background:
    b. be stopped and restarted independently from BitcoinRpcServer

TestServer::new(): spawns both servers and runs in background
TestServer::index(): only stops ord::Server and then indexes the blocks before returning
TestServer::stop_ord(): stop only ord::Server
TestServer::start_ord(): starts only ord::Server

@raphjaph
Copy link
Collaborator Author

raphjaph commented Sep 21, 2022

I converted some tests but am blocked on fixing the error below. I tried to debug for over an hour but didn't make any progress.

---- subcommand::server::tests::height_updates stdout ----
thread 'subcommand::server::tests::height_updates' panicked at 'assertion failed: self.live_write_transaction.lock().unwrap().is_none()', /Users/raphael/.cargo/registry/src/github.com-1ecc6299db9ec823/redb-0.6.1/src/db.rs:387:9
stack backtrace:
   0: rust_begin_unwind
             at /rustc/4b91a6ea7258a947e59c6522cd5898e7c0a6a88f/library/std/src/panicking.rs:584:5
   1: core::panicking::panic_fmt
             at /rustc/4b91a6ea7258a947e59c6522cd5898e7c0a6a88f/library/core/src/panicking.rs:142:14
   2: core::panicking::panic
             at /rustc/4b91a6ea7258a947e59c6522cd5898e7c0a6a88f/library/core/src/panicking.rs:48:5
   3: redb::db::Database::begin_write
             at /Users/raphael/.cargo/registry/src/github.com-1ecc6299db9ec823/redb-0.6.1/src/db.rs:387:9
   4: ord::index::Index::index_ranges
             at ./src/index.rs:152:19
   5: ord::subcommand::server::tests::TestServer::get
             at ./src/subcommand/server.rs:573:21
   6: ord::subcommand::server::tests::height_updates
             at ./src/subcommand/server.rs:853:20
   7: ord::subcommand::server::tests::height_updates::{{closure}}
             at ./src/subcommand/server.rs:850:3
   8: core::ops::function::FnOnce::call_once
             at /rustc/4b91a6ea7258a947e59c6522cd5898e7c0a6a88f/library/core/src/ops/function.rs:248:5
   9: core::ops::function::FnOnce::call_once
             at /rustc/4b91a6ea7258a947e59c6522cd5898e7c0a6a88f/library/core/src/ops/function.rs:248:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.


failures:
    subcommand::server::tests::height_updates

Its really weird because sometimes it passes:

test subcommand::server::tests::height_updates ... ok

When I run cargo test it usually passes but with cargo test --bin ord it usually fails. Much confusion.

@raphjaph
Copy link
Collaborator Author

Are tests executed in parallel?

@casey casey marked this pull request as ready for review September 22, 2022 19:29
@casey casey enabled auto-merge (squash) September 22, 2022 20:08
@casey casey merged commit 3bb4200 into ordinals:master Sep 22, 2022
@raphjaph raphjaph deleted the convert-to-unit-test branch September 27, 2022 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants