Skip to content
This repository has been archived by the owner on Dec 18, 2023. It is now read-only.

Added MemberData to query tests and fixed some failures (With a big caveat) #63

Merged
merged 4 commits into from
Sep 27, 2022

Conversation

Du-z
Copy link
Collaborator

@Du-z Du-z commented Sep 27, 2022

Downgraded xUnit from 2.4.0 to 2.4.2 to enable more tests #62
Added more test case, fixing issues as they come up.

There is a big conflict though:-
When I insert a object with a default value I would expect that I can query the DB with that default value and get back the object. But with DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingDefault these default values are not saved into the DB and therefor cannot be queried.

Changing to DefaultIgnoreCondition = JsonIgnoreCondition.Never fixes this issue, but then the RPC Create starts to fail with the following error.

Code: -32000
Message: There was a problem with the database: The table does not exist.

I have found debugging the RPC connection quite tricky. Any thoughts on how to resolve this?

@ProphetLamb
Copy link
Collaborator

Thanks for the PR! I am looking into it. On that node, ill implement ILogger support, to allow for easier debugging. I agree that it can be quite difficult to find out what the RPC client is sending

@ProphetLamb
Copy link
Collaborator

The error relates to the signin process. Where a strong typed structure is used in order to authenticate. We set Username and Password, but the class contains more fields. When not ignoring null values the following request is made:

{
  "id": "1Xh2D2i0",
  "method": "signin",
  "params": [
    { "ns": null, "db": null, "sc": null, "user": "root", "pass": "root" }
  ]
}

This is obviously invalid. For now, I'll remove the strong typed authentication structure. That should fix the specific issue, others of similar nature may occur

@ProphetLamb
Copy link
Collaborator

Fixed!

Copy link
Collaborator

@ProphetLamb ProphetLamb left a comment

Choose a reason for hiding this comment

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

The test cases are looking good to me. @Du-z Huge thank you for the good coverage, I learned quite a bit about XUnit thanks to you <3. Thought I think we are missing testcases for queries with multiple/list results, we should have just about every type dependent scenario covered. That's an amazing milestone!

@ProphetLamb ProphetLamb merged commit bb61e62 into master Sep 27, 2022
@ProphetLamb ProphetLamb deleted the query_tests_member_data branch September 27, 2022 16:25
@Du-z
Copy link
Collaborator Author

Du-z commented Sep 27, 2022

Cheers, my TODO list of tests now includes.

  • Change the Query tests to test all the DB operations not just queries.
  • Multiple result set handing
  • DB Error Responses

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants