-
Notifications
You must be signed in to change notification settings - Fork 668
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
Add a debuger function to check wrapper content #1461
Add a debuger function to check wrapper content #1461
Comments
Hi! This is an interesting suggestion, we're currently focused on releasing By the way, you could log your wrapped elements in a (arguably more) convoluted way: wrapper.findAll('p').wrappers.forEach(wrapper => console.log(wrapper.html())) or even console.log([...wrapper.findAll('p').wrappers.map(wrapper => wrapper.html())]) it is not nice, but we'll do the trick 😇 |
Is a bit tricky and hard to read, but it's for debugging so it's right for me.
Given that you're busy and since it doesn't seem like a very complex functionality. Is it okay if I try to implement it, or it's better to wait until v1.0 is released? |
I’d be happy to see this in a PR personally. Thank you for the idea and for
offering!
…On Mon, Mar 9, 2020 at 7:04 PM Sergi ***@***.***> wrote:
it is not nice, but we'll do the trick 😇
Is a bit tricky and hard to read, but it's for debugging so it's right for
me.
we're currently focused on releasing v1.0 but we'll keep it in mind for
future releases :)
Given that you're busy and since it doesn't seem like a very complex
functionality. Is it okay if I try to implement it, or it's better to wait
until v1.0 is released?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#1461?email_source=notifications&email_token=AAVL4BEITGGQMQEDU3JEIDLRGVYWRA5CNFSM4LD6WI4KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEOJMCIQ#issuecomment-596820258>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAVL4BCAZ56FBNTAGE77NXTRGVYWRANCNFSM4LD6WI4A>
.
|
What exactly is the spec for If that is the case, rather than add a new method, could we just do something like For wrapper array you can just do |
You're right, at this point the debugging function doesn't bring much more than a pretty. But it helps to encapsulate this functionality and improve it in next phases, adding more useful information. |
If we are going to add additional debugging (I think this might be hard, depending what you want to support) I think we should probably decide exactly what we want to add before we do so. If you still think this is a essential feature, can you do a proposal for what we should (and should not) add? My personal feeling is that if people want to debug, they should use a real debugger, which will do the job much better. Any reason not to just do that? |
Maybe is not essential, probably is more a nice-to-have, but I think it's simple and could add a lot of value. It's possible that to add additional debugging may be complicated and unnecesary and as you said, you should use a debugger. I was talking about more simple details (e.g. array length) In any case 'debug' can bring advantages over a pretty print. Some of them could be:
All of them could be added as function parameters in future steps. |
My thoughts:
It sounds like we are more or less reinventing what tests runners already do here, but I am open to changing my mind with some examples. One thing I would really like to see if printing the HTML output of a failed test based on a global configuration variable, like your original proposal. Something like
0 would be what we have now. 1 could automatically print the HTML and variables on |
In the end what we would be doing is printing formatted html code (and maybe additional information). Sometimes this code can be too long to read comfortably in the console, so it would be nice to save this code in a file to read it comfortably.
MaxLength is a patch for the same problem. Instead of print all the code, we can print some of it to get an idea of what we're dealing with.
Bad choice of words 😂.
vm.data is a perfect example of useful information that could be added in addition to the pretty print On the other hand I am not totally against a solution like I guess calling it When I have time I will add some examples of a possible, more definitive, implementation. |
I look forward to your design spec - I'd recommend we figure out the final API design before coding too much - it would be a shame to work on something then not get it merged due to complexity or some other issue. I'm personally still not sold on this - I think these are mostly solved problems by a regular debugging tool and a test runner in the majority of cases, but still happy to see what comes of this discussion. |
Hi everyone, I've been thinking about this for the last few days trying to get a better defininition of this hypotetical function. I will try to give a clearer definition both of the purpose of the function and of those cases in which it might be useful. I would also like to give some visual examples of a possible output and the parameters that the function could use. Objective: (Maybe When it could be useful I can think of two key cases:
Posible output After some thought, I think the most useful data would be
Other possible elements to add could be computed properties or styles, but I think that would increase the size of the overview too much. A posible output with for this information could be something like this: wrapper.overview()
// output
Wrapper (Visible):
Html:
<div class="test">
<p>My name is Tess Ting</p>
</div>
Data: {
"firstName": "Tess",
"lastName": "Ting"
}
Emitted: {
"foo": [
1. ["hello", "world"],
2. ["bye", "world"]
} The information is organized and that makes it easier to locate the main points that our test would attack. Create your own overview Posible example // We will considere this default configuration
// config.overview = {
// 'visibility': true
// 'html': true,
// 'data': true,
// 'emitted': true
//}
wrapper.overview({visibility: false, html: false, emitted: false})
// Output
Wrapper:
Data: {
"firstName": "Tess",
"lastName": "Ting"
} I hope this helps to clarify a little the reason for this issue. |
Thoughts:
|
I thought about using the 'visible' function of the wrapper itself. Wouldn't that work?
Yeah, it's probably unnecessary, showing everything should be fine.
Great, it's a clearer name, better to avoid confusion.
It's something I was thinking about while I was defining the specification. The less maintenance it needs, the better.
For me this should be a final version. Something more complex is not an overview. Well, if it's okay with you, I'll get to work on it when I get a chance. I should keep the PR open and update it or close it and open a new one to avoid confusion? |
You can use Go for it; you can open a new PR or use the existing one, up to you. I don't think this adds much maintenance overhead since it's basically built using the existing public API. |
* feat(test-utils): add 'overview' function close #1461 * feat(test-utils): add error wrapper overview * fix(test-utils): fix flow errors * fix(test-utils): compatibility with older versions * fix(test utils): flow error * feat: overview function PR comments Co-authored-by: root <[email protected]>
What problem does this feature solve?
Actually, if you try to see the contents of your wrapper the result is a little bit confusing.
It could work when wrappers from simple elements, but if biger it tursn unreadable:
One posible alternative could be to use
.html()
before log the content, but this function doesen't work with array wrappers.My proposal is to add a function to the wrappers that allows to visualize its content in a clear and simple way, without worrying about what kind of wrapper it is. A well formated pretty print of the html of the wrapper element, who gives support to array wrappers too.
It can be very useful to understand what is really being tested, especially for people in their first steps who are still learning how the library works.
What does the proposed API look like?
For simple wrapper:
For array wrapper:
The text was updated successfully, but these errors were encountered: