Skip to content

First pass at tooling to allow examples to be tested #623

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

Merged
merged 1 commit into from
Sep 10, 2022

Conversation

jskeet
Copy link
Contributor

@jskeet jskeet commented Sep 9, 2022

Very rough and ready, and will need to be expanded (e.g. for expected exceptions) but it's enough to at least start a discussion.

Not yet tested on non-Windows platforms.

Towards #614.

Very rough and ready, and will need to be expanded (e.g. for expected exceptions) but it's enough to at least start a discussion.

Not yet tested on non-Windows platforms.
@jskeet jskeet requested a review from BillWagner September 9, 2022 08:56
@jskeet
Copy link
Contributor Author

jskeet commented Sep 9, 2022

Note that I've deliberately chosen Json.NET as it's pleasantly (in this case) lax about JSON - so we don't need to add quotes around the property keys, for example. Nothing is set in stone though.

Sample output:

Deleting old output subdirectories (13)
Loaded 2 templates
Loaded 13 examples
Processing example AbstractMethodImplementation from classes.md:72-89
Processing example DirectBaseClass from classes.md:191-193
Processing example GenericBaseClass from classes.md:205-207
Processing example TypeParameterUsedAsBaseClass from classes.md:220-230
Processing example RecursiveBaseClassSpecification from classes.md:244-250
Processing example DirectBaseClasses from classes.md:262-266
Processing example SelfBaseClass from classes.md:280-281
Processing example CircularBaseClass1 from classes.md:287-290
Processing example CircularBaseClass2 from classes.md:296-301
Processing example NestedClassDependency from classes.md:313-317
Processing example DeriveFromSealedClass from classes.md:328-330
Processing example TypeParameterSubstitution from classes.md:859-882
Processing example PropertyReservedSignatures from classes.md:1295-1324
Finished example extraction.

Testing AbstractMethodImplementation from classes.md:72-89
Testing DirectBaseClass from classes.md:191-193
Testing GenericBaseClass from classes.md:205-207
Testing TypeParameterUsedAsBaseClass from classes.md:220-230
Testing RecursiveBaseClassSpecification from classes.md:244-250
Testing DirectBaseClasses from classes.md:262-266
Testing SelfBaseClass from classes.md:280-281
Testing CircularBaseClass1 from classes.md:287-290
Testing CircularBaseClass2 from classes.md:296-301
Testing NestedClassDependency from classes.md:313-317
Testing DeriveFromSealedClass from classes.md:328-330
Testing TypeParameterSubstitution from classes.md:859-882
Testing PropertyReservedSignatures from classes.md:1295-1324

Tests: 13
Failures: 0

Copy link
Member

@BillWagner BillWagner left a comment

Choose a reason for hiding this comment

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

This is a good start @jskeet

Let's :shipit:

@jskeet
Copy link
Contributor Author

jskeet commented Sep 9, 2022

Just before we merge, could you double check (maybe perhaps with @RexJaeschke or @Nigel-Ecma as well) that the few changes I've made inside the examples are okay? I think just around line 224 in classes.md would be the bit to check.

Copy link
Contributor

@Nigel-Ecma Nigel-Ecma left a comment

Choose a reason for hiding this comment

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

I don't see any issue with the renaming in the code ~224 in classes.md

As to the rest I've more questions than answers at present. I haven't looked at the tool's code. However I noted in emails that this hasn't been tested cross-platform yet – is that important for this? As Microsoft error codes are embedded in the MD this isn't going to be cross-compiler, but I'd say that's fine for a tool we're running internally. I do wonder though whether we should be embedding Microsoft specific error codes in the Standard's MD source, or at least documenting that these are not part of the Standard per se but just there for TG2 internal tooling?

@jskeet
Copy link
Contributor Author

jskeet commented Sep 10, 2022

However I noted in emails that this hasn't been tested cross-platform yet – is that important for this?

It will be, yes - because we'll want to run it as a GitHub action. I don't anticipate it being a problem, but we'll want to test it.

I do wonder though whether we should be embedding Microsoft specific error codes in the Standard's MD source, or at least documenting that these are not part of the Standard per se but just there for TG2 internal tooling?

I suggest we document in a README either in the top level or under standard/. I don't think we need to refer to it in the standard itself, given that it's only relevant for those viewing the source. (There are other comments that are only present for our Word converter already, of course.)

@jskeet jskeet merged commit 4772b7a into dotnet:draft-v7 Sep 10, 2022
@jskeet jskeet deleted the build-tooling branch September 10, 2022 07:28
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.

4 participants