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

feat(#203): RunException access modifier set to public. Docs updated #364

Merged
merged 3 commits into from
Oct 18, 2024

Conversation

mmisztal1980
Copy link
Contributor

@mmisztal1980 mmisztal1980 commented Oct 16, 2024

As per discussion in #203 . This is a simple improvement in order to make the exception assertable by testing frameworks such as FluentAssertions using:

var action = () => 
{
  // (...) // provision failing pulumi resources here
};

action.Should().Throw<Pulumi.RunException>();

This will significantly improve testing scenarios where exceptions are being thrown, as everything gets wrapped in an internal Pulumi.RunException while testing:

// Disassembled code
internal static async Task<(ImmutableArray<Resource> Resources, Exception? Exception)> TryTestAsync(
            IMocks mocks, Func<IRunner, Task<int>> runAsync, TestOptions? options = null)
        {
            var engine = new MockEngine();
            var monitor = new MockMonitor(mocks);
            await CreateRunnerAndRunAsync(() => new Deployment(engine, monitor, options), runAsync).ConfigureAwait(false);
            Exception? err = engine.Errors.Count switch
            {
                1 => new RunException(engine.Errors.Single()), // <-- wrapping takes place here
                var v when v > 1 => new AggregateException(engine.Errors.Select(e => new RunException(e))),
                _ => null
            };
            return (Resources: monitor.Resources.ToImmutableArray(), Exception: err);
        }

@mmisztal1980 mmisztal1980 requested a review from a team as a code owner October 16, 2024 10:37
Copy link
Contributor

@tgummerer tgummerer left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. I'll let someone with more context approve and merge this, but do have a small suggestion for a changelog improvement (see also https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md#changelog-messages for how we write our changelog messages)

.changes/unreleased/Improvements-364.yaml Outdated Show resolved Hide resolved
@mmisztal1980
Copy link
Contributor Author

@tgummerer when do You think someone will get the chance to look at this? :)

@Zaid-Ajaj
Copy link
Contributor

Hi there @mmisztal1980 thanks a lot for the contribution 🙏 we discussed this with the team and we thought it made sense to make the exception public.

@mmisztal1980
Copy link
Contributor Author

@Zaid-Ajaj awesome!
When is the next package release due to happen?

@Zaid-Ajaj Zaid-Ajaj added this pull request to the merge queue Oct 18, 2024
@Zaid-Ajaj
Copy link
Contributor

When is the next package release due to happen?

@mmisztal1980 Not planned at the moment, but we publish dev sdks from commits merged into main so you will be able to use the an alpha package containing your changes

Merged via the queue into pulumi:main with commit 8528c57 Oct 18, 2024
17 checks passed
@mmisztal1980 mmisztal1980 deleted the feature/203/run-exception branch October 18, 2024 16:39
@julienp julienp mentioned this pull request Nov 21, 2024
@pulumi-bot
Copy link

This PR has been shipped in release v3.69.0.

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.

4 participants