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

Update test message check #7236

Closed
wants to merge 1 commit into from
Closed

Update test message check #7236

wants to merge 1 commit into from

Conversation

ironage
Copy link
Contributor

@ironage ironage commented Jan 5, 2024

The test "bad query after bad query" occasionally fails with the message Unsupported query for table "TopLevel": key "non_queryable_field" is not a queryable field. From what I can tell, this might be a race on the server but our tests shouldn't care too much about the exact wording on the client side so I'm just adding this to the client test to accommodate.

@ironage ironage requested review from michael-wb and kiburtse January 5, 2024 20:06
@ironage ironage self-assigned this Jan 5, 2024
@kiburtse
Copy link
Contributor

kiburtse commented Jan 5, 2024

How it would help? The 'reason' in failed test log looks partially garbage, no?

4: flx: query on non-queryable field results in query error message
4:   Bad query after bad query
4: -------------------------------------------------------------------------------
4: /mnt/jenkins/workspace/realm_realm-core_PR-7229@3/test/object-store/sync/flx_sync.cpp:1801
4: ...............................................................................
4: 
4: /mnt/jenkins/workspace/realm_realm-core_PR-7229@3/test/object-store/sync/flx_sync.cpp:1776: FAILED:
4: explicitly with message:
4:   �����U
4: 
4: 
4: 
4: 
4: 
4: 
4: 
4: 
4: 
4: nsupported query for table "TopLevel": key
4:   "non_queryable_field" is not a queryable field

it's as if a few delimiters and non-printable symbols were put into random parts of the original string

Copy link

Pull Request Test Coverage Report for Build james.stone_454

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.002%) to 91.798%

Totals Coverage Status
Change from base Build 1952: -0.002%
Covered Lines: 232909
Relevant Lines: 253719

💛 - Coveralls

@ironage
Copy link
Contributor Author

ironage commented Jan 5, 2024

@kiburtse I think the garbage in the message is coming from concurrent printing to the console. If the garbage is indeed in the error message string itself, then we have a different problem and we will get the same failure again later after this is merged. So if this change doesn't fix the test, at least we have a bit more information. I couldn't reproduce this locally.

@kiburtse
Copy link
Contributor

kiburtse commented Jan 8, 2024

so it fails on fourth test run with exactly the same error. That means it's definitely not error type related. Also every output line is properly prepended with "4: ", so it's testing output prefix, doesn't look like concurrent output.

The second condition in "if" should still cover our case, it's just that there is also some weird symbols in between. Can we verify that this message comes from the server in this form?

@michael-wb
Copy link
Contributor

@ironage - can you try rerunning these tests to see if the failure is still occurring on Jenkins, now that we're not using the docker anymore?

@ironage
Copy link
Contributor Author

ironage commented Jan 18, 2024

I don't recall seeing this test fail on evergreen. Maybe this was a server error that had been fixed in recent releases and because Jenkins wasn't using the correct docker image we weren't picking up the fix. See #7248. I'll close this and assume it was fixed. We can open another issue if the problem comes up again.

@ironage ironage closed this Jan 18, 2024
@ironage ironage deleted the js/test-msg branch January 18, 2024 01:38
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants