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

Update log tests #35577

Merged
merged 3 commits into from Sep 9, 2022
Merged

Conversation

sadym-chromium
Copy link
Contributor

  • Remove implementation-defined tests. log.entryAdded handler in step 5.2 says:

If arg is a primitive ECMAScript value, append ToString(arg) to text. Otherwise append an implementation-defined string to text.

  • Switch from classic current_time to BiDi current_time_bidi.

Copy link
Contributor

@juliandescottes juliandescottes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Would be great to still test objects against any_string, and for the fixture we can simply migrate the current one to bidi.

webdriver/tests/support/fixtures_bidi.py Outdated Show resolved Hide resolved
@sadym-chromium
Copy link
Contributor Author

Thanks! Would be great to still test objects against any_string, and for the fixture we can simply migrate the current one to bidi.

@juliandescottes done, PTAL

@whimboo
Copy link
Contributor

whimboo commented Aug 26, 2022

@sadym-chromium as it looks like some tests are failing with Chrome. Even it may be unrelated to your changes maybe you could have a look? Thanks.

Copy link
Contributor

@juliandescottes juliandescottes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, as @whimboo said it would be good to know why there were chrome failures, but looking at the tests that failed I can't really see any relation with this patch?

@sadym-chromium
Copy link
Contributor Author

sadym-chromium commented Aug 29, 2022

The logging in Chromium was not implemented correctly. The PR GoogleChromeLabs/chromium-bidi#243 should address it. The problem is the BiDi implementation in ChromeDriver is updated only with ChromeDriver itself, which happens once a month.

@juliandescottes
Copy link
Contributor

I imagine @whimboo was referring to the CI failures on /webdriver/tests/bidi/browsing_context/navigate/error.py and /webdriver/tests/get_active_element/get.py ?

@whimboo
Copy link
Contributor

whimboo commented Aug 31, 2022

Maybe there were some CI issues. Lets retrigger the tests again.

@whimboo whimboo closed this Aug 31, 2022
@whimboo whimboo reopened this Aug 31, 2022
@sadym-chromium
Copy link
Contributor Author

sadym-chromium commented Sep 6, 2022

I guess we can push the change, and fix the navigation tests in ChromeDriverBiDi separately.

Copy link
Contributor

@whimboo whimboo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fine with me. Thanks.

Maksim Sadym added 2 commits September 7, 2022 11:09
* Remove implementation-defined tests (only text of primitives is defined).
* Switch from classic `current_time` to BiDi `current_time_bidi`.
@foolip
Copy link
Member

foolip commented Sep 9, 2022

Admin merging to ignore wpt-chrome-dev-stability failure, as requested.

@foolip foolip merged commit 5266695 into web-platform-tests:master Sep 9, 2022
@sadym-chromium sadym-chromium deleted the current_time branch September 21, 2022 14:56
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

6 participants