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

rego/rego_test: test with test server, not httpbin.org #5098

Conversation

srenatus
Copy link
Contributor

@srenatus srenatus commented Sep 7, 2022

Follow-up to #5097.

Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
@srenatus
Copy link
Contributor Author

srenatus commented Sep 7, 2022

Trivial. Will merge when green.

@srenatus srenatus merged commit b7bdd61 into open-policy-agent:main Sep 7, 2022
@srenatus srenatus deleted the sr/topdown/update-test-for-builtin-nbdcache branch September 7, 2022 07:39
@@ -2112,19 +2115,23 @@ p {
// Catches issues around iteration with ND builtins.
func TestNDBCacheWithRuleBodyAndIteration(t *testing.T) {
ctx := context.Background()
ts := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {
Copy link
Member

Choose a reason for hiding this comment

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

Is there some unique identifier in each response, or how do we ensure that each of them will count in the set? I guess the date header would be one, but it's not too granular, is it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The sets are keyed with the arguments to the built-in. So we'll get three items because there are three different URLs used below.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, is the results set not keyed by the responses?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it is. But the assertion checks some internals of NDBCache:

	// Ensure that the cache exists, and has exactly 3 entries.
	entries, ok := ndBC["http.send"]
	if !ok {
		t.Fatalf("expected http.send cache entry")
	}
	if entries.Len() != 3 {
		t.Fatalf("expected 3 http.send cache entries, received:\n%v", ndBC)
	}

Copy link
Member

Choose a reason for hiding this comment

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

Ah, understood 👍 I imagined the tests were checking the actual set count in-policy. Should have read better :)

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.

None yet

2 participants