CSHARP-5423: Remove deprecated field from GridFS unified tests #1579

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

Jump to bottom

Merged

papafe merged 8 commits into mongodb:main from papafe:csharp5423

Dec 30, 2024

Merged

CSHARP-5423: Remove deprecated field from GridFS unified tests #1579

papafe merged 8 commits into mongodb:main from papafe:csharp5423

Dec 30, 2024

Conversation

@papafe

Copy link

Contributor

@papafe papafe commented Dec 19, 2024

No description provided.

@papafe papafe requested a review from BorisDog

December 19, 2024 12:07

@papafe papafe requested a review from a team as a code owner

December 19, 2024 12:07

@papafe

Copy link

Contributor Author

papafe commented Dec 19, 2024

Not sure if we want to keep them, but for now I've moved the legacy tests inside the "legacy" folder.

rstam

rstam requested changes

Dec 19, 2024

View reviewed changes

Copy link

Contributor

@rstam rstam left a comment

Choose a reason for hiding this comment

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

Please make the commit tree linear before we review.

papafe added 4 commits

December 19, 2024 17:27

@papafe


 CSHARP-5423: Remove deprecated field from GridFS unified tests

72c2ff7

@papafe


 Added unified test specifications

9d8f74c

@papafe


 Small fixes

8d5e714

@papafe


 Removed yml

086f0b7

@papafe papafe force-pushed the csharp5423 branch from d8715a1 to 086f0b7 Compare

December 19, 2024 16:27

@papafe papafe requested a review from rstam

December 19, 2024 16:28

@papafe

Copy link

Contributor Author

papafe commented Dec 19, 2024

@rstam Rebased on main

@papafe papafe marked this pull request as draft

December 19, 2024 18:20

@papafe

Copy link

Contributor Author

papafe commented Dec 19, 2024

Converted it to draft as I need to finish a couple of things

@papafe


 Removed unnecessary

@papafe papafe marked this pull request as ready for review

December 20, 2024 11:26

@papafe

Copy link

Contributor Author

papafe commented Dec 20, 2024 •

edited

Loading

@BorisDog regarding the deprecated fields, namely "contentType" and "aliases", I've removed them from the registered members of the serializers and also from a test method input given they were not used.
Apart from that, no need to do more, since all the extra members are just kept into GridFSFileInfo.BackingDocument (a BsonDocument). This test verifies that:

mongo-csharp-driver/tests/MongoDB.Driver.Tests/GridFS/GridFSFileInfoTests.cs

Lines 72 to 95 in 476af2b

[Theory]

[ParameterAttributeData]

public void ExtraElements_should_be_deserialized_correctly(

[Values(new string[0], new[] { "x" }, new[] { "x", "y" }, new[] { "ExtraElements" })]

string[] names)

{

var document = CreateFilesCollectionDocument();

var extraElements = new BsonDocument();

var value = 1;

foreach (var name in names)

{

extraElements.Add(name, value++);

}

document.Merge(extraElements, overwriteExistingElements: false);

var subject = DeserializeFilesCollectionDocument(document);

foreach (var element in extraElements)

{

subject.BackingDocument[element.Name].Should().Be(element.Value);

}

@papafe


 Removed unused

749653e

BorisDog

BorisDog requested changes

Dec 21, 2024

View reviewed changes

Copy link

Contributor

@BorisDog BorisDog left a comment

Choose a reason for hiding this comment

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

Looks good overall.
There was some confusion with the legacy tests, as I thought the spec contains both legacy and unified folders.
We need to align with the spec tests structure, and have just unified tests (and remove legacy runners).
So we need to do CSHARP-3587 as prerequisite. We could also address CSHARP-3587 as part of this ticket.