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
xanlpz opened this Issue Jul 30, 2017 · 8 comments

Comments

Projects
None yet
5 participants
@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

This comment has been minimized.

Copy link
Contributor

rwaldron commented Jul 31, 2017

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

@littledan

This comment has been minimized.

Copy link
Member

littledan commented Aug 1, 2017

@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

This comment has been minimized.

Copy link
Member

leobalter commented Aug 3, 2017

@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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Member

leobalter commented Nov 10, 2017

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

@mroch those should be covered as well.

@leobalter leobalter closed this Nov 10, 2017

@littledan

This comment has been minimized.

Copy link
Member

littledan commented Nov 11, 2017

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

This comment has been minimized.

Copy link
Member

leobalter commented Nov 13, 2017

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

@littledan

This comment has been minimized.

Copy link
Member

littledan commented Jan 13, 2019

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