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

Port codec package tests to quicktest #291

Merged
merged 1 commit into from
Nov 11, 2021
Merged

Port codec package tests to quicktest #291

merged 1 commit into from
Nov 11, 2021

Conversation

masih
Copy link
Member

@masih masih commented Nov 10, 2021

Implement NodeContentEquals quicktest checker that checks two given
nodes have equal content by comparing their printout using
printer.Sprint. This simplifies the tests that use
datamode.DeepEqual by performing an equivalent check while producing a
human-readable error when nodes are not equal. The naming for this check
is inspired by a similar equality check in quicktest, named
ContentEquals.

Port the tests in codec package to quicktest; use:

  • qt.Assert for wish.Require
  • qt.Check for wish.Wish
  • qt.IsTrue for ShouldEqual over true
  • qt.IsFalse for ShouldEqual over false
  • qt.IsNil for ShouldEqual over nil
  • NodeContentEquals for ShouldEqual over nodes
  • NodeContentEquals for datamodel.DeepEqual over nodes

Update NodeContentEquals for datamodel.DeepEqual over nodes in
node package while at it.

Port node/mixins/TestSplitExact over to quicktest missed out in
earlier PRs. Note, to assert equality CmpEqual is used with an
Exporter that exports all unexpored fields.

Address TODO in node tests by using NodeContentEquals to check node
equality.

Relates to:

Depends on:

@masih masih marked this pull request as ready for review November 10, 2021 12:32
@masih masih requested review from warpfork and mvdan November 10, 2021 12:32
node/tests/checkers.go Outdated Show resolved Hide resolved
node/tests/checkers.go Outdated Show resolved Hide resolved
node/tests/checkers_test.go Outdated Show resolved Hide resolved
node/tests/testcase.go Show resolved Hide resolved
node/tests/checkers.go Outdated Show resolved Hide resolved
codec/dagjson/roundtrip_test.go Outdated Show resolved Hide resolved
codec/dagjson/roundtrip_test.go Outdated Show resolved Hide resolved
@@ -1,14 +1,16 @@
package dagjson
package dagjson_test
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌

@masih
Copy link
Member Author

masih commented Nov 10, 2021

Tests fail because #294 is not merged yet. Once that's merged, this branch needs a rebase and we are good to go 🚀

@mvdan
Copy link
Contributor

mvdan commented Nov 10, 2021

Said PR merged, if you want to rebase.

@masih
Copy link
Member Author

masih commented Nov 11, 2021

OK now we are hitting this:

Implement `NodeContentEquals` quicktest checker that checks two given
nodes have equal content by comparing their printout using
`printer.Sprint`. This simplifies the tests that use
`datamode.DeepEqual` by performing an equivalent check while producing a
human-readable error when nodes are not equal. The naming for this check
is inspired by a similar equality check in quicktest, named
`ContentEquals`.

Port the tests in `codec` package to quicktest; use:
 - `qt.Assert` for `wish.Require`
 - `qt.Check` for `wish.Wish`
 - `qt.IsTrue` for `ShouldEqual` over `true`
 - `qt.IsFalse` for `ShouldEqual` over `false`
 - `qt.IsNil` for `ShouldEqual` over `nil`
 - `NodeContentEquals` for `ShouldEqual` over nodes
 - `NodeContentEquals` for `datamodel.DeepEqual` over nodes

Update `NodeContentEquals` for `datamodel.DeepEqual` over nodes in
`node` package while at it.

Port `node/mixins/TestSplitExact` over to quicktest missed out in
earlier PRs. Note, to assert equality `CmpEqual` is used with an
`Exporter` that exports all unexpored fields.

Address TODO in node tests by using `NodeContentEquals` to check node
equality.

Relates to:
 - #219

Depends on:
 - #294
@masih
Copy link
Member Author

masih commented Nov 11, 2021

Printer issues fixed in:

Copy link
Collaborator

@warpfork warpfork left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sweet!

@masih masih merged commit b73c659 into master Nov 11, 2021
@masih masih deleted the masih/qt-port-codec branch November 11, 2021 14:23
@mvdan mvdan mentioned this pull request Nov 19, 2021
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.

3 participants