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

Switch tests to the Rust Frontegg Mock #634

Merged
merged 5 commits into from
Aug 21, 2024
Merged

Conversation

bobbyiliev
Copy link
Contributor

As per the #461 epic, now that we have all of the required endpoints transferred over, removing the Frontegg Mock Golang implementation in favour of the Rust Frontegg mock service from the main Materialize repo.

@bobbyiliev bobbyiliev marked this pull request as ready for review August 21, 2024 13:42
@bobbyiliev bobbyiliev requested a review from a team as a code owner August 21, 2024 13:42
@bobbyiliev bobbyiliev requested review from RobinClowers and removed request for a team August 21, 2024 13:42
@bobbyiliev bobbyiliev marked this pull request as draft August 21, 2024 13:43
@bobbyiliev bobbyiliev marked this pull request as ready for review August 21, 2024 13:43
@bobbyiliev bobbyiliev closed this Aug 21, 2024
@bobbyiliev bobbyiliev reopened this Aug 21, 2024
Comment on lines 30 to 31
// TODO: Mock services need to be updated to return these values
// resource.TestCheckResourceAttrSet("materialize_scim_config.scim_config_example", "tenant_id"),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As there is no official documentation for the scim endpoints, we reversed engineered the functionality.

In the old mock we used to have a placeholder for this:

newConfig.TenantID = "mockTenantID"

Will submit a follow up PR to fix this in the Rust mock service as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PR with a fix for this: MaterializeInc/materialize#29149

// Data source tests
resource.TestCheckResourceAttrPair("data.materialize_user.user_data", "id", "materialize_user.example_user", "id"),
resource.TestCheckResourceAttr("data.materialize_user.user_data", "email", email),
resource.TestCheckResourceAttr("data.materialize_user.user_data", "verified", "false"),
resource.TestCheckResourceAttr("data.materialize_user.user_data", "verified", "true"),

Choose a reason for hiding this comment

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

Is this just a change in the mock behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link

@RobinClowers RobinClowers left a comment

Choose a reason for hiding this comment

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

Awesome to see this land, I'm sure it was a ton of work!

bobbyiliev added a commit to MaterializeInc/materialize that referenced this pull request Aug 21, 2024
Fixing a small bug where the `tenant_id` in the scim configuration
should not be expected as payload but be extracted from the access token
itself.

This was noticed while switching mock service for the Terraform provider
tests in here:
MaterializeInc/terraform-provider-materialize#634
@bobbyiliev bobbyiliev merged commit f51faf1 into main Aug 21, 2024
4 checks passed
@bobbyiliev bobbyiliev deleted the switch-to-rust-frontegg-mock branch August 21, 2024 19:52
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.

2 participants