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

Removing dead code unit tests and BinFmt enablement #9126

Merged
merged 3 commits into from
Aug 21, 2023

Conversation

JanKrivanek
Copy link
Member

Fixes #8925

Context

Following types serialization is no more facilitated by BinFmt - so removing associated unit tests:

  • InvalidProjectFileException - all BuildExceptionBase exceptions tested via TestSerializationOfBuildExceptions; plus InvalidProjectFileException has it's own test: TestInvalidProjectFileException_NestedWithStack
  • LoggerException - same as above
  • ProjectStartedEventArgs - serialized via translatable only
  • AssemblyName_Ex - serialized via translatable only. The edited test file already contained tests for both types of serialization
  • CopyOnWriteDictionary - not custom serialized on any codepath (possibly only via TranslateDictionary taking IDictionary)

Changes Made

Appart from removing serialization tests of above mentioned types - removed the EnableUnsafeBinaryFormatterSerialization which was needed just because of those tests

@JanKrivanek
Copy link
Member Author

@rokonec - can you have a look on fail of Build_WithCustomBuildArgs_NetCore?

System.NotSupportedException: BinaryFormatter serialization and deserialization are disabled within this application.

I thought this should have been caught - correct?

@JanKrivanek
Copy link
Member Author

@rokonec - can you have a look on fail of Build_WithCustomBuildArgs_NetCore?

System.NotSupportedException: BinaryFormatter serialization and deserialization are disabled within this application.

I thought this should have been caught - correct?

Discussed offline - this is actually a wrong test (as it explicitly opts out from the checking functionality but do not assert on the BinFmt exception)

@JanKrivanek JanKrivanek merged commit 9ae833e into dotnet:main Aug 21, 2023
@JanKrivanek JanKrivanek deleted the proto/remove-binfmt-enablement branch August 21, 2023 13:08
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.

[BinFmt] Rework/Remove unit tests using BinaryFormatter
3 participants