chore: remove underscores from private properties and methods in some fields #6977
Conversation
| test('JS Constructor', function() { | ||
| const field = new Blockly.FieldImage('src', 10, 10, null, this.onClick); | ||
| chai.assert.equal(field.clickHandler_, this.onClick); | ||
| chai.assert.equal(field.clickHandler, this.onClick); | ||
| }); | ||
| test('setOnClickHandler', function() { | ||
| const field = new Blockly.FieldImage('src', 10, 10); | ||
| field.setOnClickHandler(this.onClick); | ||
| chai.assert.equal(field.clickHandler_, this.onClick); | ||
| chai.assert.equal(field.clickHandler, this.onClick); | ||
| }); | ||
| test('Remove Click Handler', function() { | ||
| const field = new Blockly.FieldImage('src', 10, 10, null, this.onClick); | ||
| field.setOnClickHandler(null); | ||
| chai.assert.isNull(field.clickHandler_); | ||
| chai.assert.isNull(field.clickHandler); | ||
| }); |
There was a problem hiding this comment.
Honestly not sure if these tests are worth maintaining given that setOnClickHandler literally just assigns a variable. (costs of doing updates like this might not be worth the benefits)
There was a problem hiding this comment.
Yeah. It's testing all of the arguments, but not in a super useful way.
I've been taking notes as I changed fields, and this is a pattern across the field tests. I think the solution is to test that the click handler is actually getting called (possibly using the stuff Eric is working on?) and then delete the tests that check whether the property was set.
There was a problem hiding this comment.
But it is testing that all ways of constructing this particular type of field work exactly the same, and I think that coverage is worthwhile. I'd like to preserve that, just in a way that doesn't pry inside the field so aggressively.
The basics
npm run formatandnpm run lintThe details
Resolves
Part of #6548
Proposed Changes
Remove underscores from private property and method names. Fix uses in tests. In some cases, update tests to use existing getters.