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

maackle's TODO sweep #1152

Merged
merged 3 commits into from Dec 6, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/fixt/src/lib.rs
Expand Up @@ -719,7 +719,7 @@ macro_rules! wasm_io_fixturator {
macro_rules! enum_fixturator {
( $enum:ident, $empty:expr ) => {
use rand::seq::IteratorRandom;
use $crate::prelude::strum::IntoEnumIterator;
use $crate::prelude::IntoEnumIterator;
fixturator!(
$enum,
$empty,
Expand Down
9 changes: 0 additions & 9 deletions crates/hc_bundle/src/init.rs
Expand Up @@ -95,13 +95,4 @@ mod tests {

// TODO: make these functions able to take an arbitrary stream so that
// they can be tested

// use super::*;

// #[tokio::test]
// async fn can_init_dna() {
// let tmpdir = tempdir::TempDir::new("hc_bundle").unwrap();
// init_dna(tmpdir.path().join("app")).await.unwrap();
// init_dna(tmpdir.path().join("app/n")).await.unwrap();
// }
}
2 changes: 1 addition & 1 deletion crates/holo_hash/src/hashed.rs
Expand Up @@ -11,7 +11,7 @@ use crate::PrimitiveHashType;
/// Represents some piece of content along with its hash representation, so that
/// hashes need not be calculated multiple times.
/// Provides an easy constructor which consumes the content.
// TODO: consider making lazy with OnceCell
// MAYBE: consider making lazy with OnceCell
#[cfg_attr(feature = "serialization", derive(Debug, Serialize, Deserialize))]
pub struct HoloHashed<C: HashableContent> {
/// Whatever type C is as data.
Expand Down
2 changes: 1 addition & 1 deletion crates/holochain/src/conductor.rs
Expand Up @@ -11,7 +11,7 @@

#![deny(missing_docs)]

// TODO: clean up allows once parent is fully documented
// TODO: clean up allow(missing_docs) once parent is fully documented

pub mod api;
mod cell;
Expand Down
2 changes: 0 additions & 2 deletions crates/holochain/src/conductor/cell.rs
Expand Up @@ -914,7 +914,6 @@ impl Cell {
}

/// Instantiate a Ribosome for use by this Cell's workflows
// TODO: reevaluate once Workflows are fully implemented (after B-01567)
pub(crate) async fn get_ribosome(&self) -> CellResult<RealRibosome> {
match self.conductor_api.get_dna(self.dna_hash()) {
Some(dna) => Ok(RealRibosome::new(dna)),
Expand All @@ -923,7 +922,6 @@ impl Cell {
}

/// Accessor for the authored database backing this Cell
// TODO: reevaluate once Workflows are fully implemented (after B-01567)
pub(crate) fn authored_env(&self) -> &DbWrite<DbKindAuthored> {
&self.space.authored_env
}
Expand Down
8 changes: 3 additions & 5 deletions crates/holochain/src/conductor/conductor/tests.rs
Expand Up @@ -581,7 +581,6 @@ async fn test_signing_error_during_genesis_doesnt_bork_interfaces() {
.await
.unwrap();

// TODO: match the errors more tightly
assert_matches!(response, AdminResponse::Error(_));
let response = make_signing_call(&mut app_client, &cell2).await;

Expand Down Expand Up @@ -793,14 +792,13 @@ async fn test_bad_entry_validation_after_genesis_returns_zome_call_error() {
);
}

// TODO: we need a test with a failure during a validation callback that happens
// *inline*. It's not enough to have a failing validate_create_entry for
// instance, because that failure will be returned by the zome call.
//
// NB: currently the pre-genesis and post-genesis handling of panics is the same.
// If we implement [ B-04188 ], then this test will be made more possible.
// Otherwise, we have to devise a way to discover whether a panic happened
// during genesis or not.
// NOTE: we need a test with a failure during a validation callback that happens
// *inline*. It's not enough to have a failing validate_create_entry for
// instance, because that failure will be returned by the zome call.
#[tokio::test(flavor = "multi_thread")]
#[ignore = "need to figure out how to write this test"]
async fn test_apps_disable_on_panic_after_genesis() {
Expand Down
2 changes: 1 addition & 1 deletion crates/holochain/src/conductor/entry_def_store.rs
Expand Up @@ -59,7 +59,7 @@ pub(crate) async fn get_entry_def_from_ids(
#[tracing::instrument(skip(dna))]
/// Get all the [EntryDef] for this dna
pub(crate) fn get_entry_defs(
dna: DnaFile, // TODO: make generic
dna: DnaFile,
) -> EntryDefStoreResult<Vec<(EntryDefBufferKey, EntryDef)>> {
let invocation = EntryDefsInvocation;

Expand Down
10 changes: 0 additions & 10 deletions crates/holochain/src/conductor/error.rs
Expand Up @@ -48,9 +48,6 @@ pub enum ConductorError {
#[error("Attempted to call into the conductor while it is shutting down")]
ShuttingDown,

#[error("Miscellaneous error: {0}")]
Todo(String),

#[error("Error while performing IO for the Conductor: {0}")]
IoError(#[from] std::io::Error),

Expand Down Expand Up @@ -127,13 +124,6 @@ impl ConductorError {
}
}

// TODO: can this be removed?
impl From<String> for ConductorError {
fn from(s: String) -> Self {
ConductorError::Todo(s)
}
}

impl From<one_err::OneErr> for ConductorError {
fn from(e: one_err::OneErr) -> Self {
Self::other(e)
Expand Down
6 changes: 1 addition & 5 deletions crates/holochain/src/conductor/handle.rs
Expand Up @@ -372,7 +372,7 @@ pub trait ConductorHandleT: Send + Sync {
transition: AppStatusTransition,
) -> ConductorResult<(InstalledApp, AppStatusFx)>;

// TODO: would be nice to have methods for accessing the underlying Conductor,
// MAYBE: would be nice to have methods for accessing the underlying Conductor,
// but this trait doesn't know the concrete type of underlying Conductor,
// and using generics seems problematic with mockall::automock.
// Something like this would be desirable, but ultimately doesn't work.
Expand Down Expand Up @@ -748,10 +748,6 @@ impl<DS: DnaStore + 'static> ConductorHandleT for ConductorHandleImpl<DS> {
.await;
}

// This event does not have a single Cell as a target, so we handle
// it at the conductor level.
// TODO: perhaps we can do away with the assumption that each event
// is meant for a single Cell, i.e. allow batching in general
HolochainP2pEvent::QueryOpHashes {
dna_hash,
window,
Expand Down
15 changes: 9 additions & 6 deletions crates/holochain/src/conductor/manager/mod.rs
Expand Up @@ -47,7 +47,7 @@ pub enum TaskKind {
/// but continue running the rest of the conductor and other managed tasks.
DnaCritical(Arc<DnaHash>),
/// A generic callback for handling the result
// TODO: B-01455: reevaluate whether this should be a callback
// MAYBE: B-01455: reevaluate whether this should be a callback
Generic(OnDeath),
}

Expand Down Expand Up @@ -219,7 +219,7 @@ async fn run(
context
);

// TODO: it could be helpful to modify this function so that when providing Some(app_ids),
// MAYBE: it could be helpful to modify this function so that when providing Some(app_ids),
// you can also pass in a PausedAppReason override, so that the reason for the apps being paused
// can be set to the specific error message encountered here, rather than having to read it from
// the logs.
Expand Down Expand Up @@ -258,7 +258,7 @@ async fn run(
context
);

// TODO: it could be helpful to modify this function so that when providing Some(app_ids),
// MAYBE: it could be helpful to modify this function so that when providing Some(app_ids),
// you can also pass in a PausedAppReason override, so that the reason for the apps being paused
// can be set to the specific error message encountered here, rather than having to read it from
// the logs.
Expand Down Expand Up @@ -395,13 +395,13 @@ mod test {
let mock_handle = MockConductorHandleT::new();
let (send_task_handle, main_task) = spawn_task_manager(Arc::new(mock_handle));
let handle = tokio::spawn(async {
Err(ConductorError::Todo("This task gotta die".to_string()).into())
Err(ConductorError::Other(anyhow::anyhow!("This task gotta die").into()).into())
});
let handle = ManagedTaskAdd::generic(
handle,
Box::new(|result| match result {
Ok(_) => panic!("Task should have died"),
Err(ManagedTaskError::Conductor(ConductorError::Todo(_))) => {
Err(ManagedTaskError::Conductor(ConductorError::Other(_))) => {
let handle = tokio::spawn(async { Ok(()) });
let handle = ManagedTaskAdd::ignore(handle, "respawned task");
TaskOutcome::NewTask(handle)
Expand Down Expand Up @@ -441,7 +441,10 @@ mod test {
.send(ManagedTaskAdd::unrecoverable(
tokio::spawn(async {
tokio::time::sleep(std::time::Duration::from_secs(1)).await;
Err(ConductorError::Todo("Unrecoverable task failed".to_string()).into())
Err(
ConductorError::Other(anyhow::anyhow!("Unrecoverable task failed").into())
.into(),
)
}),
"",
))
Expand Down
2 changes: 1 addition & 1 deletion crates/holochain/src/conductor/state.rs
Expand Up @@ -200,6 +200,6 @@ impl AppInterfaceConfig {
}

// TODO: Tons of consistency check tests were ripped out in the great legacy code cleanup
// We need to add these back in when we've landed the new Dna format
// We should add these back in when we've landed the new Dna format
// See https://github.com/holochain/holochain/blob/7750a0291e549be006529e4153b3b6cf0d686462/crates/holochain/src/conductor/state/tests.rs#L1
// for all old tests
2 changes: 1 addition & 1 deletion crates/holochain/src/core/queue_consumer.rs
Expand Up @@ -38,7 +38,7 @@ use holochain_types::prelude::*;
use holochain_zome_types::CellId;
use tokio::sync::{self, broadcast};

// TODO: move these to workflow mod
// MAYBE: move these to workflow mod
mod integrate_dht_ops_consumer;
use integrate_dht_ops_consumer::*;
mod sys_validation_consumer;
Expand Down
2 changes: 1 addition & 1 deletion crates/holochain/src/core/ribosome.rs
Expand Up @@ -102,7 +102,7 @@ pub enum HostContext {
GenesisSelfCheck(GenesisSelfCheckHostAccess),
Init(InitHostAccess),
MigrateAgent(MigrateAgentHostAccess),
PostCommit(PostCommitHostAccess), // TODO: add emit_signal access here?
PostCommit(PostCommitHostAccess), // MAYBE: add emit_signal access here?
ValidateCreateLink(ValidateLinkHostAccess),
Validate(ValidateHostAccess),
ValidationPackage(ValidationPackageHostAccess),
Expand Down
1 change: 0 additions & 1 deletion crates/holochain/src/core/ribosome/guest_callback/init.rs
Expand Up @@ -75,7 +75,6 @@ pub enum InitResult {
/// no init failed but some zome has unresolved dependencies
/// ZomeName is the first zome that has unresolved dependencies
/// Vec<EntryHash> is the list of all missing dependency addresses
// TODO: MD: this is probably unnecessary
UnresolvedDependencies(ZomeName, Vec<AnyDhtHash>),
}

Expand Down
2 changes: 1 addition & 1 deletion crates/holochain/src/core/ribosome/host_fn.rs
Expand Up @@ -45,7 +45,7 @@ host_fn_api_impls! {
// ------------------------------------------------------------------
// These definitions are copy-pasted from
// holochain_zome_types::zome_io
// TODO: is there a way to unhygienically import this code in both places?
// MAYBE: is there a way to unhygienically import this code in both places?

// Info about the calling agent.
fn agent_info (()) -> zt::info::AgentInfo;
Expand Down
16 changes: 10 additions & 6 deletions crates/holochain/src/core/ribosome/host_fn/capability_grants.rs
Expand Up @@ -17,8 +17,8 @@ pub fn capability_grants(
#[cfg(feature = "slow_tests")]
pub mod wasm_test {
use crate::fixt::ZomeCallHostAccessFixturator;
use crate::sweettest::SweetDnaFile;
use crate::{conductor::ConductorBuilder, sweettest::SweetConductor};
use crate::{sweettest::SweetDnaFile};
use ::fixt::prelude::*;
use hdk::prelude::*;
use holochain_types::fixt::CapSecretFixturator;
Expand All @@ -35,7 +35,8 @@ pub mod wasm_test {
let host_access = fixt!(ZomeCallHostAccess, Predictable);

let _: CapSecret =
crate::call_test_ribosome!(host_access, TestWasm::Capability, "cap_secret", ()).unwrap();
crate::call_test_ribosome!(host_access, TestWasm::Capability, "cap_secret", ())
.unwrap();
}

#[tokio::test(flavor = "multi_thread")]
Expand All @@ -44,15 +45,18 @@ pub mod wasm_test {
let host_access = fixt!(ZomeCallHostAccess, Predictable);

let secret: CapSecret =
crate::call_test_ribosome!(host_access, TestWasm::Capability, "cap_secret", ()).unwrap();
crate::call_test_ribosome!(host_access, TestWasm::Capability, "cap_secret", ())
.unwrap();
let header: HeaderHash = crate::call_test_ribosome!(
host_access,
TestWasm::Capability,
"transferable_cap_grant",
secret
).unwrap();
)
.unwrap();
let maybe_element: Option<Element> =
crate::call_test_ribosome!(host_access, TestWasm::Capability, "get_entry", header).unwrap();
crate::call_test_ribosome!(host_access, TestWasm::Capability, "get_entry", header)
.unwrap();

let entry_secret: CapSecret = match maybe_element {
Some(element) => {
Expand All @@ -67,7 +71,7 @@ pub mod wasm_test {
assert_eq!(entry_secret, secret,);
}

// TODO: [ B-03669 ] can move this to an integration test (may need to switch to using a RealDnaStore)
// MAYBE: [ B-03669 ] can move this to an integration test (may need to switch to using a RealDnaStore)
#[tokio::test(flavor = "multi_thread")]
async fn ribosome_authorized_call() -> anyhow::Result<()> {
observability::test_run().ok();
Expand Down
8 changes: 6 additions & 2 deletions crates/holochain/src/core/ribosome/host_fn/delete.rs
Expand Up @@ -43,7 +43,12 @@ pub fn delete<'a>(
deletes_entry_address,
};
let header_hash = source_chain
.put(Some(call_context.zome.clone()), header_builder, None, chain_top_ordering)
.put(
Some(call_context.zome.clone()),
header_builder,
None,
chain_top_ordering,
)
.await
.map_err(|source_chain_error| {
WasmError::Host(source_chain_error.to_string())
Expand All @@ -65,7 +70,6 @@ pub(crate) fn get_original_address<'a>(

tokio_helper::block_forever_on(async move {
let mut cascade = Cascade::from_workspace_network(&workspace, network);
// TODO: Think about what options to use here
let maybe_original_element: Option<SignedHeaderHashed> = cascade
.get_details(address.clone().into(), GetOptions::content())
.await?
Expand Down
2 changes: 1 addition & 1 deletion crates/holochain/src/core/sys_validate/error.rs
Expand Up @@ -38,7 +38,7 @@ pub enum SysValidationError {
SourceChainError(#[from] SourceChainError),
#[error("Dna is missing for this hash {0:?}. Cannot validate without dna.")]
DnaMissing(DnaHash),
// TODO: Remove this when SysValidationResult is replace with SysValidationOutcome
// NOTE: can remove this if SysValidationResult is replaced with SysValidationOutcome
#[error(transparent)]
ValidationOutcome(#[from] ValidationOutcome),
#[error(transparent)]
Expand Down
2 changes: 1 addition & 1 deletion crates/holochain/src/core/workflow.rs
Expand Up @@ -34,7 +34,7 @@ pub mod publish_dht_ops_workflow;
pub mod sys_validation_workflow;
pub mod validation_receipt_workflow;

// TODO: either remove wildcards or add wildcards for all above child modules
// MAYBE: either remove wildcards or add wildcards for all above child modules
pub use call_zome_workflow::*;
pub use genesis_workflow::*;
pub use initialize_zomes_workflow::*;
7 changes: 2 additions & 5 deletions crates/holochain/src/core/workflow/genesis_workflow.rs
Expand Up @@ -4,9 +4,6 @@
//! - AgentId
//!

// FIXME: understand the details of actually getting the DNA
// FIXME: creating entries in the config db

use super::error::WorkflowError;
use super::error::WorkflowResult;
use crate::core::ribosome::guest_callback::genesis_self_check::{
Expand Down Expand Up @@ -81,7 +78,7 @@ where
return Err(WorkflowError::GenesisFailure(reason));
}

// TODO: this is a placeholder for a real DPKI request to show intent
// NB: this is just a placeholder for a real DPKI request to show intent
if api
.dpki_request("is_agent_pubkey_valid".into(), agent_pubkey.to_string())
.await
Expand Down Expand Up @@ -213,7 +210,7 @@ pub mod tests {
}
}

/* TODO: make doc-able
/* TODO: update and rewrite as proper rust docs

Called from:

Expand Down
2 changes: 1 addition & 1 deletion crates/holochain/src/test_utils/conductor_setup.rs
Expand Up @@ -83,7 +83,7 @@ impl CellHostFnCaller {
}

/// Everything you need to run a test that uses the conductor
// TODO: refactor this to be the "Test Conductor" wrapper
// TODO: rewrite as sweettests if possible
pub struct ConductorTestData {
_envs: TestEnvs,
handle: ConductorHandle,
Expand Down
1 change: 0 additions & 1 deletion crates/holochain/tests/inline_zome_spec.rs
Expand Up @@ -80,7 +80,6 @@ fn simple_crud_zome() -> InlineZome {
api.get(vec![GetInput::new(hash.into(), GetOptions::default())])
.map_err(Into::into)
})
// TODO: let this accept a usize, once the hdk refactor is merged
.callback("emit_signal", |api, ()| {
api.emit_signal(AppSignal::new(ExternIO::encode(()).unwrap()))
.map_err(Into::into)
Expand Down