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

Handle non-string values logged from Chrome (devtools) console #1787

Merged
merged 1 commit into from
Oct 4, 2021

Conversation

gillius
Copy link

@gillius gillius commented Oct 4, 2021

Proposed fix for #1786

Description

This is my first PR. I ensured project built locally but did not test this (yet). Still would need to figure out how to do this. But based on examination in debugger I am fairly certain this will work.

@gillius
Copy link
Author

gillius commented Oct 4, 2021

I'm not sure how to test this, the local develop would build 2.0.0 and I don't want to build a non-SNAPSHOT locally, I tested in debugger to run the logger.debug passing in a non-string value and it worked, and observed in debugger in Karate 1.1.0 that the values List did contain the non-string objects expected from a console.log(true) or console.log(1) command -- an Boolean or Integer object.

@ptrthomas ptrthomas merged commit 5b6320e into karatelabs:develop Oct 4, 2021
@ptrthomas
Copy link
Member

@gillius many thanks, I'm ok to merge without a test for this one

@gillius
Copy link
Author

gillius commented Oct 4, 2021

OK I was able to test this locally using the help of https://github.com/karatelabs/karate/wiki/Developer-Guide:

mvn versions:set -DnewVersion=2.0.0-SNAPSHOT
mvn install -P pre-release -pl -karate-robot -DskipTests=true

The unit tests did not work for me, so I needed skip tests.

Now I am able to use 2.0.0-SNAPSHOT in my projects locally, there I tested against the cucumber test provided in #1786 and it passed.

I see you merged already, but hopefully that makes it feel better.

@gillius gillius deleted the console_non_string branch October 4, 2021 19:22
@ptrthomas
Copy link
Member

@gillius gillius mentioned this pull request Oct 5, 2021
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants