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

Added tenancy tests for workload controller #19429

Closed
wants to merge 4 commits into from

Conversation

Ganeshrockz
Copy link
Contributor

@Ganeshrockz Ganeshrockz commented Oct 30, 2023

Description

  • Modified existing controller's tests to include tenancies.

Testing & Reproduction steps

Links

PR Checklist

  • updated test coverage
  • external facing docs updated
  • appropriate backport labels added
  • not a security concern

@Ganeshrockz Ganeshrockz changed the title Added tenancy information to tests Added tenancy tests for workload controller Oct 30, 2023
@Ganeshrockz Ganeshrockz added pr/no-backport pr/no-changelog PR does not need a corresponding .changelog entry labels Oct 30, 2023
Copy link
Contributor

@erichaberkorn erichaberkorn left a comment

Choose a reason for hiding this comment

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

Have you tried running this on ENT? It's probably best for PRs to start on the ENT side.

actualHealth, err := getNodeHealth(context.Background(), suite.runtime, node.Id)
require.NoError(t, err)
require.Equal(t, health, actualHealth)
func (suite *controllerSuite) runTestCaseWithTenancies(name string, t func(*pbresource.Tenancy)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It tests t typically refers to a *testing.T. Could we rename this something else?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we do something so we don't need to pass in the name of the parent test? Maybe if this passes a*testing.T, it can use a t.Run to make the subtest?

@Ganeshrockz
Copy link
Contributor Author

Closing this in favour of #19530

@Ganeshrockz Ganeshrockz closed this Nov 6, 2023
@Ganeshrockz Ganeshrockz added the consul-india PRs/Issues assigned to Consul India team label Dec 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consul-india PRs/Issues assigned to Consul India team pr/no-backport pr/no-changelog PR does not need a corresponding .changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants