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

Remove Serializers RunDescriptors #4495

Merged
merged 1 commit into from Feb 23, 2017

Conversation

timbussmann
Copy link
Contributor

I think there is no need to add capability checks to the Requires class, as serializers can be changed between tests and this is not a restriction which needs to be made for the whole test suite. Both XML and JSON are still built in, so it should be safe to just configure the endpoints with whatever is required instead of using the Repeat statement.

@Particular/nservicebus-maintainers @Particular/azure-maintainers (I think the azure repos are the only ones which don't use XML by default?) do you see any potential issue with that approach?

@andreasohlund
Copy link
Member

Looking at the external json serializer I don't get how it switches the acpt tests to use it?

https://github.com/Particular/NServiceBus.Newtonsoft.Json/tree/develop/src/AcceptanceTests

@SimonCropp will this change work for it?

@SimonCropp
Copy link
Contributor

@andreasohlund only way to do it was a hack Particular/NServiceBus.Newtonsoft.Json@4f218f8

and it looks like that change was blatted by this Particular/NServiceBus.Newtonsoft.Json#14

i have reapplied the hack here Particular/NServiceBus.Newtonsoft.Json@b7a0b2f

@andreasohlund
Copy link
Member

i have reapplied the hack here Particular/NServiceBus.Newtonsoft.Json@b7a0b2f

@timbussmann that hack will no longer work with this change right?

@timbussmann
Copy link
Contributor Author

@timbussmann that hack will no longer work with this change right?

correct. The tests specific serializer config will run at the end and override the external json serializer for those few tests which define a specific serializer.
Not sure this is an issue though? If those specific tests should run with the external serializer, shouldn't we just copy them into the packages root acceptance test folder and treat them as package specific tests?

@andreasohlund
Copy link
Member

Not sure this is an issue though? If those specific tests should run with the external serializer, shouldn't we just copy them into the packages root acceptance test folder and treat them as package specific tests?

@SimonCropp thoughts? (essentially disconnecting from the core acpt tests source package)

@SimonCropp
Copy link
Contributor

no problem with that. the acceptance tests in the context of a serializer are a fairly blunt tool

@andreasohlund
Copy link
Member

Small enough so merging this

@andreasohlund andreasohlund merged commit 519913e into develop Feb 23, 2017
@andreasohlund andreasohlund deleted the remove-serializer-descriptors branch February 23, 2017 12:38
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.

None yet

3 participants