Conversation
|
Also closes #559 |
|
Seems like the volume of data included in the commit exceeds what might be strictly necessary to test the function? |
|
I'll strip it down 👍 |
| for(let i = 0; i < nodes.length; i++){ | ||
| if(nodes[i].textContent.trim().length === 0){ | ||
| nodes[i].remove(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Do you need to loop through all to childNodes or could you get away with checking just the first and last? I'm not 100% sure how whitespace and textNodes work so you may have to loop through all of them.
There was a problem hiding this comment.
It seems that I only need to check the last one
| }); | ||
|
|
||
| test("[" + browserType + "]" + " <map-span> hides tile boundaries", async ()=>{ | ||
| const total = await page.$eval( |
There was a problem hiding this comment.
Why is the variable named "total" ? What is it the total of?
| }); | ||
|
|
||
| //https://github.com/Maps4HTML/Web-Map-Custom-Element/issues/559#issuecomment-959805896 | ||
| test("[" + browserType + "]" + " White space parsing for map-coordinates", async ()=>{ |
There was a problem hiding this comment.
Title of the test could be easier to understand, e.g. "Ensure white space in map-coordinates is ignored" or something like that.
| (path) => path.getAttribute("d") | ||
| ); | ||
|
|
||
| expect(feature).toEqual("M0 217L0 217L0 217L2 217L4 218L6 218L6 218L6 216L2 214L0 216L0 216"); |
| <map-feature id="fclass.73" class="fclass"> | ||
| <map-geometry> | ||
| <map-polygon> | ||
| <map-coordinates> |
There was a problem hiding this comment.
You could put an xml comment here that says e.g. <!-- leading and trailing whitespace should not affect parsing of map-coordinates -->
prushforth
left a comment
prushforth
left a comment
There was a problem hiding this comment.
Thanks for doing these test, it is much appreciated. Testing is the basis of civilizational progress lol. Just try to keep the semantics of what your doing up front for poor old you or whoever comes after you (often yourself, actually) who comes along and tries to figure it out later.
|
If you feel like updating anything, go ahead, but merging this so you can move on as required. |
4 participants
Closes #536