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

Reuse error functions #41

Merged
merged 3 commits into from
Jul 21, 2015
Merged

Conversation

jugglinmike
Copy link
Collaborator

This builds on gh-40 by also re-using Test262's definition of the $ERROR function.

@bterlson What is Test262's "API"? I can see two possible interpretations:

  1. Test262 test files rely on the contents of an opaque file named sta.js (which may change at any time)
  2. Test262 test files rely on global $ERROR and Test262Error functions

Although this pull request assumes #1 is true, I'm starting to think that #2 is more realistic. If that's the case, this approach should still work (just with a little more modification).

Reduce coupling between runner implementations and Test262 itself by
automatically injecting Test262's definition of `Test262Error`.
Reduce coupling between runner implementations and Test262 itself by
automatically injecting Test262's definition of `$ERROR`.
@bterlson
Copy link
Owner

I see #2 as the real case - runners have to provide the definitions for these functions because they might change depending on environment. I'm surprised tests pass with this PR...

@jugglinmike
Copy link
Collaborator Author

I'm surprised tests pass with this PR...

They pass because none of the currently-implemented runners require custom definitions of $ERROR or Test262Error. Unlike $LOG and $DONE, I can't think of a case where $ERROR or Test262Error would need to be configured. Do you have any thoughts on that?

The alternative I had in mind is to attach the definitions for these objects as properties on the base Runner prototype--then sub-classes could over-ride if necessary. Does that sound good to you?

@jugglinmike
Copy link
Collaborator Author

I've pushed up another commit to demonstrate the change I was describing in my previous comment.

@jugglinmike
Copy link
Collaborator Author

Documentation will also need to be updated, but I'll hold off on that until I hear back about the code change.

@bterlson
Copy link
Owner

My original thinking was that $ERROR could do host-specific error reporting but I guess this is not required if $DONE received the error and can report it appropriately?

@jugglinmike
Copy link
Collaborator Author

I think the main thing is that the host needs to be able to recognize uncaught errors generally (whether from expected negative cases or unexpected failures due to implementation bugs), so defining the behavior of $ERROR in a host-specific way would be of limited use.

@bterlson
Copy link
Owner

Yeah that makes sense! I like 33c083c so I'm going to pull this in.

bterlson added a commit that referenced this pull request Jul 21, 2015
@bterlson bterlson merged commit f0154d2 into bterlson:master Jul 21, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants