Skip to content

Commit

Permalink
merge: #3965
Browse files Browse the repository at this point in the history
3965: Enhance dal-test helpers and schemas r=nickgerace a=nickgerace

## Description

This PR aims to enhance `dal-test` helpers and test exclusive schemas. As a result of these changes, panics of any kind are banned by default outside of macro expansion helpers. Now, when authoring test exclusive schemas or authoring tests, error reporting should be clearer than before.

For the helpers, we leverage `color_eyre` via its `Result` type and `eyre!` macro in order to make error propagation better in tests via explicit `expect` statements and other failures. Inner panics are now disallowed in helpers themselves in order to give better error location reporting when running tests via `buck2` with default settings.

For the test exclusive schemas, everything has been refactored to be more explicit about how schemas are organized and where migrations are centralized. These changes reflect the shape of the schemas back in the old engine when they were in the dal. In addition to these changes, the helpers for test exclusive schemas have been polished, including some unnecessary usages of async functions and replaced panics. These helpers have been moved to the central module file and now, the only public, exported function is the `migrate` function. Finally, panics within test exclusive schema scaffolding have also been replaced with proper error propagation.

As a result of these changes, both `DalTestHelpersError` and `ChangeSetTestHelpers` have been removed alongside their result types.

## Example Test Failure

![image](https://github.com/systeminit/si/assets/39320683/bacd3264-b105-42ae-99bb-ee550e1ab6b5)

## Secondary Changes

- Fix swallowed errors when setting types in frame integration tests
- Split property editor test view into its own submodule
- Move auth token creation into expand helpers (moved from the regular test helpers)
- Temporarily disable `delete_frame_with_child_with_resource` test due to intermittent failures

## History

This PR is the continuation of #3964, which was purely closed because I accidentally deleted the branch.

Co-authored-by: Nick Gerace <[email protected]>
  • Loading branch information
si-bors-ng[bot] and nickgerace authored Jun 10, 2024
2 parents 91c40be + 2fda656 commit d776260
Show file tree
Hide file tree
Showing 43 changed files with 1,218 additions and 1,030 deletions.
25 changes: 18 additions & 7 deletions lib/dal-test/src/expand_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@
//! macro expansion.
use dal::{ChangeSet, ChangeSetId, DalContext, UserClaim};
use jwt_simple::algorithms::RSAKeyPairLike;
use jwt_simple::claims::Claims;
use jwt_simple::prelude::Duration;
use tracing_subscriber::{fmt, util::SubscriberInitExt, EnvFilter, Registry};

use crate::{
helpers::{create_auth_token, generate_fake_name},
WorkspaceSignup,
};
use crate::{helpers::generate_fake_name, jwt_private_signing_key, WorkspaceSignup};

/// This function is used during macro expansion for setting up a [`ChangeSet`] in an integration test.
pub async fn create_change_set_and_update_ctx(
Expand All @@ -25,7 +25,7 @@ pub async fn create_change_set_and_update_ctx(
.expect("no workspace snapshot set on base change set");
let change_set = ChangeSet::new(
ctx,
generate_fake_name(),
generate_fake_name().expect("could not generate fake name"),
Some(base_change_set_id),
workspace_snapshot_address,
)
Expand Down Expand Up @@ -142,7 +142,7 @@ pub async fn workspace_signup(ctx: &DalContext) -> crate::Result<(WorkspaceSignu

let mut ctx = ctx.clone_with_head().await?;

let workspace_name = generate_fake_name();
let workspace_name = generate_fake_name().expect("could not generate fake name");
let user_name = format!("frank {workspace_name}");
let user_email = format!("{workspace_name}@example.com");

Expand All @@ -153,6 +153,17 @@ pub async fn workspace_signup(ctx: &DalContext) -> crate::Result<(WorkspaceSignu
user_pk: nw.user.pk(),
workspace_pk: *nw.workspace.pk(),
})
.await;
.await
.expect("could not create auth token");
Ok((nw, auth_token))
}

async fn create_auth_token(claim: UserClaim) -> crate::Result<String> {
let key_pair = jwt_private_signing_key().await?;
let claim = Claims::with_custom_claims(claim, Duration::from_days(1))
.with_audience("https://app.systeminit.com")
.with_issuer("https://app.systeminit.com")
.with_subject(claim.user_pk);

Ok(key_pair.sign(claim).expect("could not sign"))
}
Loading

0 comments on commit d776260

Please sign in to comment.