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

Class fields test coverage - task list #1161

Open
28 tasks
xanlpz opened this issue Jul 30, 2017 · 8 comments
Open
28 tasks

Class fields test coverage - task list #1161

xanlpz opened this issue Jul 30, 2017 · 8 comments

Comments

@xanlpz
Copy link
Contributor

xanlpz commented Jul 30, 2017

As suggested by @littledan, here is a TODO list proposal with the tests that we should have for the class fields feature. The format is: - [checkbox] Description (asigned to)
Some doubts about this:

  • Do we need to test every type of field in every possible combination with methods in class? Seems it would be at least: a field by itself; a field before a method; a field after a method; a field between methods. The V8 tests I looked at seemed satisfied with the first two options only.
  • How much information about the spec should there be on each line? And what format to use?

Runtime

  • FieldDefinition: LiteralPropertyName Initializer; a = 0; ()
  • FieldDefinition: LiteralPropertyName Initializer; LiteralPropertyName; a = 0; b; (@xanlpz)
  • FieldDefinition: LiteralPropertyName Initializer; MethodDefinition; a = 0; b() {}; (@xanlpz)
  • FieldDefinition: LiteralPropertyName Initializer; GeneratorMethod; a = 0; *b() {}; (@xanlpz)
  • FieldDefinition: LiteralPropertyName Initializer; MethodDefinition + ComputedPropertyName; a = 0; 'b' {}; (@xanlpz)
  • FieldDefinition: LiteralPropertyName; a; (@xanlpz)
  • FieldDefinition: LiteralPropertyName; LiteralPropertyName; a; b; (@xanlpz)
  • FieldDefinition: LiteralPropertyName; MethodDefinition; a; b() {}; (@xanlpz)
  • FieldDefinition: LiteralPropertyName; GeneratorMethod; a; *b() {}; (@xanlpz)
  • FieldDefinition: LiteralPropertyName; MethodDefinition + ComputedPropertyName; a; 'b' {}; (@xanlpz)
  • FieldDefinition: ComputedPropertyName Initializer; ['a'] = 0; (@xanlpz)
  • FieldDefinition: ComputedPropertyName Initializer; LiteralPropertyName; ['a'] = 0; b; (@xanlpz)
  • FieldDefinition: ComputedPropertyName Initializer; MethodDefinition; ['a'] = 0; b() {}; (@xanlpz)
  • FieldDefinition: ComputedPropertyName Initializer; GeneratorMethod; ['a'] = 0; *b() {}; (@xanlpz)
  • FieldDefinition: ComputedPropertyName Initializer; MethodDefinition + ComputedPropertyName; ['a'] = 0; 'b' {}; (@xanlpz)
  • FieldDefinition: ComputedPropertyName; ['a']; (@xanlpz)
  • FieldDefinition: ComputedPropertyName; LiteralPropertyName; ['a']; b; (@xanlpz)
  • FieldDefinition: ComputedPropertyName; MethodDefinition; ['a']; b() {}; (@xanlpz)
  • FieldDefinition: ComputedPropertyName; GeneratorMethod; ['a']; *b() {}; (@xanlpz)
  • FieldDefinition: ComputedPropertyName; MethodDefinition + ComputedPropertyName; ['a']; 'b' {}; (@xanlpz)
  • FieldDefinition: NumericLiteral Initializer; 0 = 0; (@xanlpz)
  • FieldDefinition: NumericLiteral; 0; (@xanlpz)
  • FieldDefinition: StringLiteral Initializer; 'a' = 0; (@xanlpz)
  • FieldDefinition: StringLiteral; 'a'; (@xanlpz)

Static

  • ClassElement: static FieldDefinition; throws SyntaxError if PropName is "prototype". ()
  • ClassElement: static FieldDefinition; throws SyntaxError if PropName is "constructor". ()
  • ClassElement: FieldDefinition; throws SyntaxError if PropName is "constructor". ()
  • ClassElement: PrivateName; throws SyntaxError if PropName is "#constructor". ()

(Initial example, after we agree on a format I will continue adding examples and errors referenced in the spec)

@rwaldron
Copy link
Contributor

I'll be here to review as you work your way through this list.

@littledan
Copy link
Member

@xanlpz This format looks good. The only thing is, when you write ['b']() it seems to be rendering as a markdown link. Maybe putting backticks around your code samples will be the easiest way to solve it.

@leobalter
Copy link
Member

@xanlpz I'd suggest to assign yourself to smaller parts. I can accept fragmented PRs. This way can also lead someone else to work together on other unassigned features.


Can we move this content to #1055 and close this issue?

@mroch
Copy link
Contributor

mroch commented Aug 18, 2017

please include tests for super as well! things like

class C extends B { x = () => super() } // SyntaxError
class C extends B { x = () => super.y } // ok I think?
class C extends B { x = super.y } // ???
class C extends B { super } // ok, valid IdentifierName

@leobalter
Copy link
Member

Closing this as we got class-fields pretty much well covered.

@mroch those should be covered as well.

@littledan
Copy link
Member

Is there a test for multiple public fields of the same name? (The semantics should be that all initializers run, and the second definition clobbers the first; the earlier definition should be visible while intermediate initializers are running.)

@leobalter
Copy link
Member

I'll give a check and fix the coverage for this if necessary.

@littledan
Copy link
Member

There's been a lot of tests added for class fields; I think we can consider this task complete.

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

No branches or pull requests

5 participants