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 encoding of generic type when boxing #33

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bruno-cadorette
Copy link

Fix #32

@bruno-cadorette
Copy link
Author

@MangelMaxime The tests are failing on this build but I'm not seeing where the tests are in the solution. I may have other fixes for this library so I'd like to be able to run them on my machine.

Like this one is failing:
All/Thoth.Json.Encode/Basic/Encode.Auto.toString generate null if skipNullField is true and the optional field value of type classes is None

https://dev.azure.com/thoth-org/Thoth.Json.Net/_build/results?buildId=211&view=logs&j=43d32c17-ff19-5d0a-58b3-52a09bf40d57&t=b22a0aeb-72b1-5841-e6d8-921de89fbb6e

@MangelMaxime
Copy link
Contributor

The test are pulled from Thoth.Json repo using paket.

By default, you should be able to run the test on your machine because paket should pull the dependencies. It is a bit more tricky if you want to modify the tests.

In general, I implement the new feature/fix in Thoth.Json because that's the repo with the tests and then I update Thoth.Json.Net code with the new dependencies.

To update the dependencies you will need to update:

  • paket.dependencies
  • build.fsx

Basically the inverse of this commit 26e0d88

I know it is not really user friendly but I didn't have the time yet to use gitmodule in order to make it easier.

If you just want to test your fix against this repo you can modify the files under paket-files/Tests/... but be careful because these files are not saved so be sure to save your progress before re-installing the project, etc.

@bruno-cadorette
Copy link
Author

sorry for the late response I didn't had time to run the command
When I run dotnet test on the src folder, I get an empty output (even dotnet test -t doesn't output anything)
I am not really familiar with paket or using dotnet as a command line

@MangelMaxime
Copy link
Contributor

In order, to run the test you should use fake build: fake.cmd build -t Test

Source file

If you want to run the test manually:

  1. Run fake.cmd build -t AdaptTest (only needed once in order to adapt the namespace in thje files)
  2. Run dotnet build tests -f netcoreapp3.0
  3. Run dotnet tests/bin/Release/netcoreapp3.0/Tests.dll"

I didn't test the commands, but that should be something like by reading to build.fsx source code. But it is just easier to run the test from Fake because it does all the job for you :)

@bruno-cadorette
Copy link
Author

thanks, I was able to make them run!

@MangelMaxime
Copy link
Contributor

I tried to port it to the next version of Thoth.Json libraries and indeed this fix generates several errors.

Unfortunately, my understanding of Reflection code is limited and for now, I don't know how to handle it.

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.

Cannot serialized higher order type when type is boxed
2 participants