Skip to content

Conversation

@facumenzella
Copy link
Contributor

@facumenzella facumenzella commented Oct 7, 2025

Add APITests for the new RevenueCatUI package

@facumenzella facumenzella force-pushed the feat/integrationtests branch 2 times, most recently from 9730f48 to 316d70e Compare October 7, 2025 10:23
@facumenzella facumenzella added the pr:other Changes to our CI configuration files and scripts label Oct 7, 2025
@facumenzella facumenzella force-pushed the feat/integrationtests branch from 316d70e to 2f1094a Compare October 7, 2025 10:29
@facumenzella facumenzella marked this pull request as ready for review October 7, 2025 10:58
@facumenzella facumenzella requested a review from a team as a code owner October 7, 2025 10:58
@facumenzella facumenzella mentioned this pull request Oct 7, 2025
Copy link
Contributor

@tonidero tonidero left a comment

Choose a reason for hiding this comment

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

A couple of comments. Main one is about making sure this is not included in the final bundle for the RevenueCatUI library.

public void PaywallOptions_OfferingIdentifierConstructor_SetsOfferingIdentifier()
{
const string offeringId = "test_offering";
var options = new PaywallOptions(offeringId);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is going to change to accept an offering instead of an ID with #710 right?


namespace RevenueCatUI.Tests
{
public class PaywallOptionsAPITests
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm it's interesting since we don't have tests for the RevenueCat module... are we sure this won't be included in the final bundle? For RevenueCat, we have the API tests live in the IntegrationTests project instead. Maybe we could follow the same approach by making that project also depend on RevenueCatUI and add the tests there?

@facumenzella
Copy link
Contributor Author

I'll do the same as for RevenueCat in another branch 👍

@facumenzella
Copy link
Contributor Author

#735

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:other Changes to our CI configuration files and scripts

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants