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

Create a test for #11264 #11281

Closed
jdm opened this issue May 19, 2016 · 5 comments
Closed

Create a test for #11264 #11281

jdm opened this issue May 19, 2016 · 5 comments
Labels

Comments

@jdm
Copy link
Member

@jdm jdm commented May 19, 2016

It's not clear to me why it's difficult to test. We had a reproducible test case, and the code changes seem to be working around a particular situation that can occur. Why can't we reverse-engineer the conditions that make that code be executed?

cc @KiChjang

@jdm jdm added the A-testing label May 19, 2016
@KiChjang
Copy link
Member

@KiChjang KiChjang commented May 19, 2016

I'm really not sure why the logs in #11261 show that there was a .borrow() call from update_href before. I double-scanned and triple-scanned htmlanchorelement.rs before #11264 was merged, and I still don't see any borrow() at all in update_href.

I wasn't able to reproduce it on my computer since I'm using Linux and I hit a FontCache panic.

@jdm
Copy link
Member Author

@jdm jdm commented May 24, 2016

I have not been able to reproduce #11261 yet from 904e9ea. The panic backtrace reported by @paulrouget doesn't make sense at all, any @KiChjang wasn't able to reproduce the failure so the fix was produced on faith. I would like to properly figure out the actual problem we solved; @paulrouget can you revert to the revision in which you observed the panic and get a proper backtrace from rust_panic on a debug build?

@nox
Copy link
Member

@nox nox commented Oct 1, 2017

Was this done?

@nox nox added the C-is-this-done label Oct 1, 2017
@jdm
Copy link
Member Author

@jdm jdm commented Oct 1, 2017

It was not done.

@jdm jdm removed the C-is-this-done label Oct 1, 2017
@jdm
Copy link
Member Author

@jdm jdm commented Mar 30, 2020

This will never be done.

@jdm jdm closed this Mar 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.