Skip to content

Commit

Permalink
Add regression test for existing code gen funcs and new variants
Browse files Browse the repository at this point in the history
This commit adds a regression test for attaching existing code gen funcs
to new schema variants. Specifically, the "attaching" step works, but
when the "Secrets" input is added, sometimes we are unable to regenerate
the variant due the following error: "pkg error: Can't convert identity
to LeafInputLocation".

The bug appears to no longer exist on main, but the test should still be
valuable in catching a future regression.

Secondary changes:
- Rename "find_by_name" to "find_id_by_name" when looking for funcs by
  name

Signed-off-by: Nick Gerace <[email protected]>
  • Loading branch information
nickgerace committed Jun 11, 2024
1 parent d776260 commit 9184a62
Show file tree
Hide file tree
Showing 16 changed files with 332 additions and 220 deletions.
4 changes: 2 additions & 2 deletions lib/dal/src/func.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ impl Func {
Ok(Self::assemble(func_node_weight, &inner))
}

pub async fn find_by_name(
pub async fn find_id_by_name(
ctx: &DalContext,
name: impl AsRef<str>,
) -> FuncResult<Option<FuncId>> {
Expand Down Expand Up @@ -514,7 +514,7 @@ impl Func {

pub async fn find_intrinsic(ctx: &DalContext, intrinsic: IntrinsicFunc) -> FuncResult<FuncId> {
let name = intrinsic.name();
Self::find_by_name(ctx, name)
Self::find_id_by_name(ctx, name)
.await?
.ok_or(FuncError::IntrinsicFuncNotFound(name.to_owned()))
}
Expand Down
2 changes: 1 addition & 1 deletion lib/dal/src/func/authoring/create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ async fn create_func_stub(
handler: &str,
) -> FuncAuthoringResult<Func> {
let name = name.unwrap_or(generate_name());
if Func::find_by_name(ctx, &name).await?.is_some() {
if Func::find_id_by_name(ctx, &name).await?.is_some() {
return Err(FuncAuthoringError::FuncNameExists(name));
}

Expand Down
4 changes: 2 additions & 2 deletions lib/dal/src/pkg/export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -940,7 +940,7 @@ impl PkgExporter {
let intrinsic_name = intrinsic.name();
// We need a unique id for intrinsic funcs to refer to them in custom bindings (for example
// mapping one prop to another via si:identity)
let intrinsic_func_id = Func::find_by_name(ctx, intrinsic_name)
let intrinsic_func_id = Func::find_id_by_name(ctx, intrinsic_name)
.await?
.ok_or(PkgError::MissingIntrinsicFunc(intrinsic_name.to_string()))?;

Expand Down Expand Up @@ -1021,7 +1021,7 @@ impl PkgExporter {
async fn export_intrinsics(&mut self, ctx: &DalContext) -> PkgResult<Vec<FuncSpec>> {
let mut funcs = vec![];
for instrinsic in IntrinsicFunc::iter() {
let intrinsic_func_id = Func::find_by_name(ctx, instrinsic.name()).await?.ok_or(
let intrinsic_func_id = Func::find_id_by_name(ctx, instrinsic.name()).await?.ok_or(
PkgError::MissingIntrinsicFunc(instrinsic.name().to_string()),
)?;

Expand Down
6 changes: 3 additions & 3 deletions lib/dal/src/pkg/import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ async fn import_change_set(
// packages. We also apply this to si:resourcePayloadToValue since it should be an
// intrinsic but is only in our packages
if func::is_intrinsic(func_spec.name()) || SPECIAL_CASE_FUNCS.contains(&func_spec.name()) {
if let Some(func_id) = Func::find_by_name(ctx, func_spec.name()).await? {
if let Some(func_id) = Func::find_id_by_name(ctx, func_spec.name()).await? {
let func = Func::get_by_id_or_error(ctx, func_id).await?;

thing_map.insert(unique_id.to_owned(), Thing::Func(func.to_owned()));
Expand Down Expand Up @@ -903,7 +903,7 @@ pub async fn clone_and_import_funcs(
let func = if func::is_intrinsic(func_spec.name())
|| SPECIAL_CASE_FUNCS.contains(&func_spec.name())
{
let func_id = Func::find_by_name(ctx, &func_spec.name())
let func_id = Func::find_id_by_name(ctx, &func_spec.name())
.await?
.ok_or(PkgError::MissingIntrinsicFunc(func_spec.name().to_owned()))?;

Expand Down Expand Up @@ -1617,7 +1617,7 @@ pub async fn attach_resource_payload_to_value(
ctx: &DalContext,
schema_variant_id: SchemaVariantId,
) -> PkgResult<()> {
let func_id = Func::find_by_name(ctx, "si:resourcePayloadToValue")
let func_id = Func::find_id_by_name(ctx, "si:resourcePayloadToValue")
.await?
.ok_or(PkgError::FuncNotFoundByName(
"si:resourcePayloadToValue".into(),
Expand Down
Loading

0 comments on commit 9184a62

Please sign in to comment.