Add heading block test

[JavonDavis]

Part of #745

To test: Run all the device tests, ensure they pass

Update release notes:

• If there are user facing changes, I have added an item to RELEASE-NOTES.txt.

[peril-wordpress-mobile]

	Fails
🚫	Danger failed to run /app/danger-0.zxn9g1vb8w.ts.

Error Error

ENOENT: no such file or directory, open '/app/danger-0.zxn9g1vb8w.ts'

Error: ENOENT: no such file or directory, open '/app/danger-0.zxn9g1vb8w.ts'
    at Object.openSync (fs.js:439:3)
    at Object.readFileSync (fs.js:344:35)
    at Object.<anonymous> (/app/node_modules/danger/distribution/runner/runners/vm2.js:106:87)
    at step (/app/node_modules/danger/distribution/runner/runners/vm2.js:43:23)
    at Object.next (/app/node_modules/danger/distribution/runner/runners/vm2.js:24:53)
    at /app/node_modules/danger/distribution/runner/runners/vm2.js:18:71
    at new Promise (<anonymous>)
    at __awaiter (/app/node_modules/danger/distribution/runner/runners/vm2.js:14:12)
    at Object.exports.runDangerfileEnvironment (/app/node_modules/danger/distribution/runner/runners/vm2.js:95:121)
    at runDangerAgainstFileInline (/app/out/danger/danger_runner.js:49:37)
    at process._tickCallback (internal/process/next_tick.js:68:7)

Dangerfile

Generated by 🚫 dangerJS

[mchowning]

Should this say that this fails if the Heading Block "is empty" instead of "is not empty"?

If this does need to be updated, would it make sense to make the comment for sendTextToHeadingBlock follow the same pattern as the comment we use here? For example, they could both be either "Fails if Heading Block is/is-not empty" or they could both have the form of "Assumes Heading Block is empty/not-empty". It wouldn't make sense to do this if both methods don't (for example) fail when they receive the wrong input, but if it does make sense, having a shared form would make the behavior a bit easier to understand quickly. I do not feel remotely strongly about this, just a thought.

[JavonDavis]

The comment is correct, when the heading block is not empty the inner element changes and then the current code won't work if that's the case. But I get what you're saying with the consistency, I've made that change 👍

[mchowning]

Thanks for explaining! Let me ask another question to help me understand.

It's not intuitive to me that a getText... method like this would only work on empty blocks. Would that not result in the method just always return an empty string (since the block is "empty")? I may just be misunderstanding what we mean by empty here.

This is probably related, but I'm also a bit confused by the empty value in the calls to getTextViewForHeadingBlock in both sendTextToHeadingBlock and this getTextForHeadingBlock. It sounds like we're assuming the Heading Block is empty in both of these methods, but we're passing different empty boolean values to getTextViewForHeadingBlock in the two methods. If we're assuming that the Heading block is empty in both methods, I thought (probably mistakenly) that we would also pass an empty value of true to getTextViewForHeadingBlock in both methods. Can you help me understand where I'm going wrong?

[JavonDavis]

@mchowning you're right here! I think my mind was still thinking of an earlier state of the code, the code works as you understand it and you'd expect it to, my bad here! 🙇

[JavonDavis]

I've gone ahead and removed the misleading comments

[JavonDavis]

@mchowning I've merged in the changes from the other PRs, if everything's still green it should be good for you to give it another 👀

[mchowning]

@JavonDavis Thanks for the update. I added a follow-up comment seeking just a bit more clarification. I think there I'm still misunderstanding how we're using empty. I'd appreciate it if you could shed some (more) light. 😀

[mchowning]

LGTM! Thanks @JavonDavis!