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

Sub groups #40

Closed
wants to merge 2 commits into from
Closed

Sub groups #40

wants to merge 2 commits into from

Conversation

twainj
Copy link
Contributor

@twainj twainj commented Jun 9, 2015

I added the new tests to verify the change to a new unit test class, as that seemed to be the best place for them.

… that has a SubstitutionGroupChildRule for it.

Effectively, the DomXmlWriter was requiring that a substitution take place for the element. But according to the standard, it is acceptable for the original element to be used (if it is not abstract).
Tests included.
…a) substitution option, and failing tests from Substitution Group fixes.

In order to change DomXmlWriter to use the most derived substitution option, the substitutions are joined to the type lineage, which in order of derivation, before selecting the first substitution in the list.
The failing tests were:
- CircuitEditorTests.EditSaveCloseAndReopen
- DiagramEditorTests.StatechartEditSaveCloseAndReopen
@twainj
Copy link
Contributor Author

twainj commented Jun 9, 2015

@Ron2 Here's the pull request we talked about. Let me know if you see changes that need to happen.

@Ron2
Copy link
Member

Ron2 commented Jun 10, 2015

Thank you very much, Jason. That was a lot of work on your part and the unit tests were well-written. I reviewed the changes and they look good but then when I ran the full test suite, another unit test failed -- ModifyChildDocumentExternally in TimelineEditorTests. I'm looking into what is going on there.

@Ron2
Copy link
Member

Ron2 commented Jun 10, 2015

Never mind about that unit test -- it fails in our current release and in our work-in-progress branch on my computer. I suspect it's somehow related to Windows 8.1 or my laptop somehow. Anyway, I merged in your changes. Thanks again!

@Ron2 Ron2 closed this Jun 10, 2015
@twainj
Copy link
Contributor Author

twainj commented Jun 10, 2015

Thanks Ron,

Glad to help. I've been really impressed with ATF, and have enjoyed
being able to contribute. Hope to do so again in the future when the
need arises!

Cheers,

Jason

On 6/9/2015 6:03 PM, Ron wrote:

Never mind about that unit test -- it fails in our current release and
in our work-in-progress branch on my computer. I suspect it's somehow
related to Windows 8.1 or my laptop somehow. Anyway, I merged in your
changes. Thanks again!


Reply to this email directly or view it on GitHub
#40 (comment).

@twainj twainj deleted the sub_groups branch June 13, 2015 00:02
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.

2 participants