feat: selector #13
feat: selector #13
Conversation
There was a problem hiding this comment.
Thank you for moving this important functionality, I have tried to take a look at this functionality in a few places in detail and it looks good.
I have also tried to integrate into oscd-subscriber-later-binding and I have quite a range of test failures after I do that which I have not yet found time to look at and if you can wait I would like to examine. I use both identity and selector as part of the UI so I feel that would be worth investigating.
And also I would like to integrate this into openscd/open-scd and check that tests pass and prepare a PR for this so that we can immediately benefit from this library.
| it('returns correct selector for all tags except IEDName and ProtNs', () => { | ||
| Object.keys(selectorTags).forEach(tag => { | ||
| const elements = Array.from(doc.querySelectorAll(tag)).filter( | ||
| item => !item.closest('Private') | ||
| ); | ||
|
|
||
| elements.forEach(element => { | ||
| if (element) | ||
| expect(element).to.satisfy( | ||
| // eslint-disable-next-line no-shadow | ||
| (element: Element) => | ||
| element === doc.querySelector(selector(tag, identity(element))) | ||
| ); | ||
| }); | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
I particularly like this "battery of tests"
There was a problem hiding this comment.
Yes, these seem very good to have! They could also catch #14 if the test data was extended to a case with ExtRefs sharing an intAddr.
There was a problem hiding this comment.
Thank you. I will add one more ExtRef element covering the edge case @danyill triggered here.
| it('returns tagName with non SCL tag', () => | ||
| expect(selector('LNodeSpec', 'someLNodeSpec')).to.equal('LNodeSpec')); | ||
|
|
||
| it('returns correct selector for all tags except IEDName and ProtNs', () => { |
There was a problem hiding this comment.
Are these really an exception now?
There was a problem hiding this comment.
True, thank you!
| it('return empty string with non SCL tag', () => | ||
| expect(parentTags('LNodeSpec').length).to.equal(0)); | ||
|
|
||
| it('return empty all possible parents for SCL tag', () => |
There was a problem hiding this comment.
| it('return empty all possible parents for SCL tag', () => | |
| it('returns all possible parents for SCL tag', () => |
|
(At least) part of the reason for my tests failing is that the ExtRef identity function in this library is different to that in open-scd. In open-scd we have: function extRefIdentity(e: Element): string | number {
if (!e.parentElement) return NaN;
const parentIdentity = identity(e.parentElement);
const iedName = e.getAttribute('iedName');
const intAddr = e.getAttribute('intAddr');
const intAddrIndex = Array.from(
e.parentElement.querySelectorAll(`ExtRef[intAddr="${intAddr}"]`)
).indexOf(e);
if (intAddr) return `${parentIdentity}>${intAddr}[${intAddrIndex}]`;
const [Here we have: Lines 86 to 97 in 2df4a18 The handling of |
|
I decided to close this PR. We cannot resolve some element purly based selector string. Those are in particular |
|
Please look into this PR for a better way to get SCL element based on an identity string, openscd/open-scd#1285. There is also a branch in preparation that does transport it to this repository. https://github.com/JakobVogelsang/oscd-scl/tree/find |
Closes #6
This time i did touch
IEDNameandProtNS. Both were not tested and for no good reason. Along the way I have increased the test coverage on some test cases such as ExtRef with intAddr and w/o and others.The test is running some time as we do the test for all element in the validScl file. I then also promoses to cover a lot of edge cases