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

Enable more BinaryFormatter tests #107408

Merged
merged 3 commits into from
Sep 6, 2024
Merged

Conversation

adamsitnik
Copy link
Member

@adamsitnik adamsitnik commented Sep 5, 2024

The idea is simple:

  • enable the BinaryFormatter tests in System.Runtime.Serialization.Formatters.Tests
  • create a new test project, disable BF and move the tests that check whether BF is disabled or enabled to this project

The SerializationGuardTests were failing, because the guard is no longer being activated:

// When BinaryFormatter was built-in to the platform we used to activate SerializationGuard here,
// but now that it has moved to an OOB offering it no longer does.

(see #103255 (comment) for details)

@adamsitnik adamsitnik added area-System.Runtime test-enhancement Improvements of test source code binaryformatter-migration Issues related to the removal of BinaryFormatter and migrations away from it labels Sep 5, 2024
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

…he OOB package, the tests need to reflect that.
{
Assembly.Load(new byte[1000]);
SerializationGuard.ThrowIfDeserializationInProgress("AllowProcessCreation", ref s_cachedSerializationSwitch);
Copy link
Member Author

Choose a reason for hiding this comment

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

So far the test was trying to start calc:

private ProcessStarter(SerializationInfo info, StreamingContext context)
{
Process.Start("calc.exe");
}

and just asserting that deserialization was failing with SerializationException:

SerializationException se = Assert.Throws<SerializationException>(() => reader.Deserialize(ms));

I decided to just call SerializationGuard.ThrowIfDeserializationInProgress here. I expect that it's going to do the job without having any side-effects.

I could have just deleted those tests, but then it would not be explicit that it's not something that we want.

@adamsitnik adamsitnik marked this pull request as ready for review September 5, 2024 16:59
@adamsitnik
Copy link
Member Author

/ba-g the OOM was fixed by #107433, it's unrelated to this PR

@adamsitnik adamsitnik merged commit 8685859 into dotnet:main Sep 6, 2024
81 of 85 checks passed
adamsitnik added a commit to adamsitnik/runtime that referenced this pull request Sep 9, 2024
* enable the BinaryFormatter tests in System.Runtime.Serialization.Formatters.Tests

* add new test project, where the flag is disabled and it runs only 3 tests in total that ensure that

* The SerializationGuard is no longer activated since BF was moved to the OOB package, the tests need to reflect that.
github-actions bot pushed a commit that referenced this pull request Sep 17, 2024
* enable the BinaryFormatter tests in System.Runtime.Serialization.Formatters.Tests

* add new test project, where the flag is disabled and it runs only 3 tests in total that ensure that

* The SerializationGuard is no longer activated since BF was moved to the OOB package, the tests need to reflect that.
carlossanlop pushed a commit that referenced this pull request Sep 17, 2024
* Remove package references from library tests (#106737)

* Remove package references from library tests

These tests should be referencing the product assemblies so that they
test latest and not old bits.

* Reference the OOB version of SRSF and make sure it's copied

* BinaryFormatter tests should be skipped only on AOT, WASM and Mobile (#106858)

* respect AppContext switch (which is currently enabled for all projects in the root Directory.Build.props file)

* add project reference to all test projects that need working BF (and were being skipped for a while)

* adjust to changes from #104202: EqualityComparer<string>.Default is mapped to StringEqualityComparer, but serialized as GenericEqualityComparer<string>

* Don't use WeakReferences in the round trip test, as the target may get freed in the meantime, fixes #104905 (#106967)

* Enable more BinaryFormatter tests (#107408)

* enable the BinaryFormatter tests in System.Runtime.Serialization.Formatters.Tests

* add new test project, where the flag is disabled and it runs only 3 tests in total that ensure that

* The SerializationGuard is no longer activated since BF was moved to the OOB package, the tests need to reflect that.

* Disable binary formatter tests when DotNetBuildSourceOnly. (#107549)

* [mono][tvos] Do not treat assembly.pdb/xml files as native files to bundle when AOTing on Helix (#107079)

* Do not treat assembly.pdb/xml files as native files to bundle

* Bundle satellite assemblies as well

* [mono][ci] Include PDBs from runtime pack when building on Helix if required (#107348)

---------

Co-authored-by: Eric StJohn <ericstj@microsoft.com>
Co-authored-by: Tom Deseyn <tom.deseyn@gmail.com>
Co-authored-by: Ivan Povazan <55002338+ivanpovazan@users.noreply.github.com>
carlossanlop pushed a commit that referenced this pull request Sep 17, 2024
…107903)

* Remove package references from library tests (#106737)

* Remove package references from library tests

These tests should be referencing the product assemblies so that they
test latest and not old bits.

* Reference the OOB version of SRSF and make sure it's copied

* BinaryFormatter tests should be skipped only on AOT, WASM and Mobile (#106858)

* respect AppContext switch (which is currently enabled for all projects in the root Directory.Build.props file)

* add project reference to all test projects that need working BF (and were being skipped for a while)

* adjust to changes from #104202: EqualityComparer<string>.Default is mapped to StringEqualityComparer, but serialized as GenericEqualityComparer<string>

* Don't use WeakReferences in the round trip test, as the target may get freed in the meantime, fixes #104905 (#106967)

* Enable more BinaryFormatter tests (#107408)

* enable the BinaryFormatter tests in System.Runtime.Serialization.Formatters.Tests

* add new test project, where the flag is disabled and it runs only 3 tests in total that ensure that

* The SerializationGuard is no longer activated since BF was moved to the OOB package, the tests need to reflect that.

* Disable binary formatter tests when DotNetBuildSourceOnly. (#107549)

* [mono][tvos] Do not treat assembly.pdb/xml files as native files to bundle when AOTing on Helix (#107079)

* Do not treat assembly.pdb/xml files as native files to bundle

* Bundle satellite assemblies as well

* [mono][ci] Include PDBs from runtime pack when building on Helix if required (#107348)

---------

Co-authored-by: Eric StJohn <ericstj@microsoft.com>
Co-authored-by: Adam Sitnik <adam.sitnik@gmail.com>
Co-authored-by: Tom Deseyn <tom.deseyn@gmail.com>
Co-authored-by: Ivan Povazan <55002338+ivanpovazan@users.noreply.github.com>
jtschuster pushed a commit to jtschuster/runtime that referenced this pull request Sep 17, 2024
* enable the BinaryFormatter tests in System.Runtime.Serialization.Formatters.Tests

* add new test project, where the flag is disabled and it runs only 3 tests in total that ensure that

* The SerializationGuard is no longer activated since BF was moved to the OOB package, the tests need to reflect that.
sirntar pushed a commit to sirntar/runtime that referenced this pull request Sep 30, 2024
* enable the BinaryFormatter tests in System.Runtime.Serialization.Formatters.Tests

* add new test project, where the flag is disabled and it runs only 3 tests in total that ensure that

* The SerializationGuard is no longer activated since BF was moved to the OOB package, the tests need to reflect that.
@github-actions github-actions bot locked and limited conversation to collaborators Oct 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Runtime binaryformatter-migration Issues related to the removal of BinaryFormatter and migrations away from it test-enhancement Improvements of test source code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants