Skip to content

Commit

Permalink
merge: #3971
Browse files Browse the repository at this point in the history
3971: Add regression test for existing code gen funcs and new variants r=nickgerace a=nickgerace

## Description

This PR 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

Co-authored-by: Nick Gerace <[email protected]>
  • Loading branch information
si-bors-ng[bot] and nickgerace authored Jun 11, 2024
2 parents 76f6aa1 + 9184a62 commit b18f9aa
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 @@ -314,7 +314,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 @@ -516,7 +516,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 b18f9aa

Please sign in to comment.