Skip to content

Conversation

@andrewstucki
Copy link
Contributor

Found out that we had an acceptance test that was being skipped due to a non-matching regex in the scenarios, this adjusts the scenario so we don't see this anymore:

You can implement step definitions for undefined steps with these snippets:
func roleShouldHaveMembersAndInCluster(arg1, arg2, arg3, arg4 string) error {
	return godog.ErrPending
}
func userShouldBeAbleToReadFromTopicInCluster(arg1, arg2, arg3 string) error {
	return godog.ErrPending
}
func InitializeScenario(ctx *godog.ScenarioContext) {
	ctx.Step(`^role "([^"]*)" should have members "([^"]*)" and "([^"]*)" in cluster "([^"]*)"$`, roleShouldHaveMembersAndInCluster)
	ctx.Step(`^user "([^"]*)" should be able to read from topic "([^"]*)" in cluster "([^"]*)"$`, userShouldBeAbleToReadFromTopicInCluster)
}

And role "admin-role" is successfully synced
Then role "admin-role" should exist in cluster "roles"
And role "admin-role" should have members "alice" and "bob" in cluster "roles"
And role "admin-role" should have members "alice and bob" in cluster "roles"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change is a bit funny, but it matches how the scenario's step definition is actually written, the regex contains a single capture group for the members string and then has this logic:

	if strings.Contains(members, " and ") {

which basically means that it's tokenizing a single quoted string around an optional and inclusion in the string itself. This should probably eventually be rewritten to not be so weird and at some point I might take a look to see if godog's step parsing logic allows you to do thing like have a +-based capture group where you can treat it like an array or something, but for now this is just to minimally get this matching what it's supposed to do.

@andrewstucki andrewstucki enabled auto-merge (squash) September 23, 2025 15:44
@chrisseto
Copy link
Contributor

Looks like this will need to be rebased @andrewstucki. You're hitting the rackwareness flake.

@andrewstucki andrewstucki merged commit c9dcbfc into main Sep 24, 2025
10 checks passed
@RafalKorepta RafalKorepta deleted the as/fix-acceptance-role-scenarios branch September 25, 2025 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants