Skip to content

chore(tests): Add regression for now passing test#8441

Merged
TomAFrench merged 7 commits intomasterfrom
mv/regression_8018
May 12, 2025
Merged

chore(tests): Add regression for now passing test#8441
TomAFrench merged 7 commits intomasterfrom
mv/regression_8018

Conversation

@vezenovm
Copy link
Contributor

@vezenovm vezenovm commented May 9, 2025

Description

Problem*

Resolves #8018

Summary*

While looking at a separate inlining panic, I was going through the other inlining bugs to see where there may be similarities. I noticed that this regression does seem to be failing on master so I have added a new test under noir_test_success.

Additional Context

Documentation*

Check one:

  • No documentation needed.
  • Documentation included in this PR.
  • [For Experimental Features] Documentation to be submitted in a separate PR.

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@vezenovm vezenovm requested a review from a team May 9, 2025 21:20
@TomAFrench
Copy link
Member

I think if we're testing compiler behaviour then we should have this in test_programs rather than in the stdlib itself.

Looks like we're using a stdlib test to test the compiler is correct rather than testing the actual stdlib implementation of that logic.

@TomAFrench
Copy link
Member

ehhhh, on second look this is a sensible testcase for a sorting algo. I was just thrown by the test being duplicated for local and global inputs. I'd probably simplify to just have one of those.

@vezenovm
Copy link
Contributor Author

vezenovm commented May 9, 2025

ehhhh, on second look this is a sensible testcase for a sorting algo. I was just thrown by the test being duplicated for local and global inputs. I'd probably simplify to just have one of those.

I switched to one test in the stdlib. Did you still want this under test_programs?

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Execution Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: b72929a Previous: 6accfc6 Ratio
rollup-merge 0.004 s 0.003 s 1.33

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Execution Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: 0cf4198 Previous: 6accfc6 Ratio
rollup-merge 0.004 s 0.003 s 1.33

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

@vezenovm
Copy link
Contributor Author

Switched to testing under noir_test_success. This also avoids snapshot changes with noir_stdlib/array/mod.nr changing.

@TomAFrench TomAFrench added this pull request to the merge queue May 12, 2025
Merged via the queue into master with commit 1b593f6 May 12, 2025
118 checks passed
@TomAFrench TomAFrench deleted the mv/regression_8018 branch May 12, 2025 13:42
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request May 13, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix: sign extend in signed cast
(noir-lang/noir#8264)
chore(fuzz): Do not use zero length types in the main input output
(noir-lang/noir#8465)
chore: fix visibility issues in test suite
(noir-lang/noir#8454)
chore: blackbox functions for ssa intepreter
(noir-lang/noir#8375)
feat: improve bitshift codegen
(noir-lang/noir#8442)
fix(ssa): Mark mutually recursive simple functions
(noir-lang/noir#8447)
fix: Fix nested trait dispatch with associated types
(noir-lang/noir#8440)
chore: carry visibilities in monomorphized AST
(noir-lang/noir#8439)
chore(tests): Add regression for now passing test
(noir-lang/noir#8441)
chore: use human-readable bytecode in snapshots
(noir-lang/noir#8164)
chore: bump external pinned commits
(noir-lang/noir#8445)
END_COMMIT_OVERRIDE

---------

Co-authored-by: AztecBot <tech@aztecprotocol.com>
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
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.

Unreachable code panic: All Value::Params should already be known from previous calls to translate_block.

2 participants