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

fix: Fix flaky tests, clean up logic on rules #596

Merged
merged 1 commit into from
Jan 31, 2023
Merged

Conversation

kentquirk
Copy link
Contributor

Which problem is this PR solving?

  • Some of the tests for rules were dependent on random numbers. This fixes that, and also cleans up some of the datatype conversion rules to work in more cases

Short description of the changes

  • Fix tests to check both the keep and rate values
  • Fix logic in some tests
  • Change tryConvertToString to just use sprintf("%v"), even for strings, for consistency
  • fix string operators to use tryConvertToString everywhere

@kentquirk kentquirk requested a review from a team as a code owner January 31, 2023 23:32
@kentquirk kentquirk requested a review from fchikwekwe January 31, 2023 23:32
@kentquirk kentquirk added type: bug Something isn't working version: no bump A PR with maintenance or doc changes that aren't included in a release. labels Jan 31, 2023
@kentquirk kentquirk changed the title bug: Fix flaky tests, clean up logic on rules fix: Fix flaky tests, clean up logic on rules Jan 31, 2023
@@ -1392,12 +1403,13 @@ func TestRulesDatatypes(t *testing.T) {
{
Event: types.Event{
Data: map[string]interface{}{
"test": 9.3,
"test": 9.3, // "9.3" is greater than "10.3" when compared as a string
Copy link
Contributor

Choose a reason for hiding this comment

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

👀

Copy link
Member

Choose a reason for hiding this comment

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

Huh. Turns out it's consistent with F# too, so this isn't just some "rob pike is a weird dude" behavior:

λ ~/scratch/otel-pixie/ dotnet fsi

Microsoft (R) F# Interactive version 12.4.0.0 for F# 7.0
Copyright (c) Microsoft Corporation. All Rights Reserved.

For help type #help;;

> "9.3" > "10.3";;
val it: bool = true

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, alphabetically '9' is greater than '1'. :)

@kentquirk kentquirk merged commit 3cc259e into main Jan 31, 2023
@kentquirk kentquirk deleted the kent.fix_flaky_tests branch January 31, 2023 23:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working version: no bump A PR with maintenance or doc changes that aren't included in a release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants