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

Too big zio-test output fix #8614

Merged
merged 11 commits into from Jan 16, 2024

Conversation

urbit-pilled
Copy link
Contributor

/claim #8579

@CLAassistant
Copy link

CLAassistant commented Jan 11, 2024

CLA assistant check
All committers have signed the CLA.

Copy link

algora-pbc bot commented Jan 11, 2024

💵 To receive payouts, sign up on Algora, link your Github account and connect with Stripe/Alipay.

@urbit-pilled
Copy link
Contributor Author

Reproducing the example issue in https://gist.github.com/evis/cd51be073b26c6a9f9ff45723d988e26 (#8579)

package com.zioShortenedExample

import zio.test._

case class ReceiptRequest(receiptContent: ReceiptContent, paymentMetadata: Option[PaymentMetadata], receiptFooter: Option[String])

case class ReceiptContent(compositeEventId: CompositeEventId, receiptType: ReceiptType, payments: List[ReceiptPayment], rows: List[ReceiptRow], agentType: AgentType, firmInn: String, firmUrl: String, firmReplyEmail: String, clientEmailOrPhone: String, taxationType: TaxationType, taxCalcMethod: Option[TaxCalcMethod], fiscalDocType: Option[FiscalDocType], supplierPhone: Option[String], serviceId: Option[String])

case class CompositeEventId(eventId: String, paymentId: String, paymentIdType: PaymentIdType)

case class ReceiptPayment(amount: Int, paymentType: PaymentType)

case class ReceiptRow(agentType: AgentType, taxType: Int, paymentTypeType: PaymentTypeType, qty: Double, price: Int, text: String, supplierInfo: Option[SupplierInfo], productType: Option[String])

case class SupplierInfo(inn: String, name: Option[String], phone: Option[String])

case class PaymentMetadata(paymentDatetime: String)

sealed trait ReceiptType
case class Income() extends ReceiptType

sealed trait AgentType
case class Agent() extends AgentType

sealed trait TaxationType
case class OSN() extends TaxationType

sealed trait TaxCalcMethod
case class Total() extends TaxCalcMethod

sealed trait FiscalDocType
case class Receipt() extends FiscalDocType

sealed trait PaymentType
case class Card() extends PaymentType

sealed trait PaymentIdType
case class VerticalsArenda() extends PaymentIdType

sealed trait PaymentTypeType
case class Prepayment() extends PaymentTypeType

object MyAppSpec extends ZIOSpecDefault {
  override def spec = suite("TestingApplicationsExamplesSpec")(
    test("reproduce") {
      val receiptRequest1 = ReceiptRequest(
        ReceiptContent(
          CompositeEventId("domainReceiptType#", "paymentId", VerticalsArenda()),
          Income(),
          List(ReceiptPayment(10, Card())),
          List(
            ReceiptRow(
              Agent(),
              0,
              Prepayment(),
              1.0,
              10,
              "text",
              Some(SupplierInfo("0000000000", Some("name"), Some("7999887766"))),
              None
            )
          ),
          Agent(),
          "firmInn",
          "firmUrl",
          "firmReplyEmail@mail.ru",
          "test@mail.ru",
          OSN(),
          Some(Total()),
          Some(Receipt()),
          Some("7999887766"),
          None
        ),
        Some(PaymentMetadata("2023-12-04T17:15:03.961393289Z")),
        None
      )

      val receiptRequest2 = ReceiptRequest(
        ReceiptContent(
          CompositeEventId("domainReceiptType#", "paymentId", VerticalsArenda()),
          Income(),
          List(ReceiptPayment(100, Card())),
          List(
            ReceiptRow(
              Agent(),
              0,
              Prepayment(),
              1.0,
              100,
              "text",
              Some(SupplierInfo("0000000000", Some("name"), Some("7999887766"))),
              None
            )
          ),
          Agent(),
          "firmInn",
          "firmUrl",
          "firmReplyEmail@mail.ru",
          "test@mail.ru",
          OSN(),
          Some(Total()),
          Some(Receipt()),
          Some("7999887766"),
          None
        ),
        Some(PaymentMetadata("2023-12-04T17:15:03.961393289Z")),
        None
      )

      assert(receiptRequest1)(Assertion.equalTo(receiptRequest2))
    }
  )
}

Should output

  - TestingApplicationsExamplesSpec / reproduce
    ✗ ReceiptRequest.receiptContent.payments[0].amount : expected '10' got '100'
ReceiptRequest.receiptContent.rows[0].price : expected '10' got '100'

@urbit-pilled
Copy link
Contributor Author

Another simpler example

import zio.test._

object MyAppSpec extends ZIOSpecDefault {
  override def spec = suite("TestingApplicationsExamplesSpec")(
    test("reproduce"){
      case class Person(name: String, age: Int, address: Address)
      case class Address(city: String, street: String)

      val person1 = Person("John", 30, Address("New York", "123 Main St"))
      val person2 = Person("John", 30, Address("San Francisco", "456 Oak St"))
      assert(person1)(Assertion.equalTo(person2))
    }
  )
}

and it's output

- TestingApplicationsExamplesSpec / reproduce
    ✗ Person$1.address.city : expected 'New York' got 'San Francisco'
Person$1.address.street : expected '123 Main St' got '456 Oak St'

@urbit-pilled urbit-pilled marked this pull request as ready for review January 12, 2024 11:10
Copy link
Contributor

@adamgfraser adamgfraser left a comment

Choose a reason for hiding this comment

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

Can we add some tests for this?

@urbit-pilled
Copy link
Contributor Author

Sure I'll add some tests, but I'm new to this repo so I had some trouble trying to work out how to write a test that compares against a failed assertions error message.

I added this to test-tests/shared/src/test/scala/zio/test/AssertionSpec.scala

test("equalTo must print out shortened value") {
  val probe: SampleUser = SampleUser("User", 50)
  val expected: SampleUser = sampleUser
  assert(probe)(equalTo(expected))
} @@failing,

This will check that the error fails with @@failing and assert, however, I do not know how to make a test that will check against the error message string.
I see there is a hasMessage assertion, but I don't know how to use that with the results of assert(probe)(equalTo(expected))

@adamgfraser
Copy link
Contributor

There is a variant of failing that accepts a function from a TestFailure to a Boolean.

@urbit-pilled
Copy link
Contributor Author

I added three tests for the shortened assertEqual error message.

@adamgfraser adamgfraser merged commit 6cb051b into zio:series/2.x Jan 16, 2024
20 checks passed
@urbit-pilled urbit-pilled deleted the simplified-zio-test-error branch January 16, 2024 17:34
@evis
Copy link
Contributor

evis commented Apr 26, 2024

@ghostdogpr could you release 2.0.x version with this fix, please? We'd like to use it, but don't want to use pre-release version 2.1.x in our service.

Or maybe, there are some plans to release stable 2.1.x soon?

@ghostdogpr
Copy link
Member

ghostdogpr commented Apr 26, 2024

Can you make a PR targeting the series/2.0.x branch?

Regarding 2.1.0, I am planning to do a last RC this weekend and a final release probably a week after if no regression was found.

case (obj1: Product, obj2: Product) if obj1.productArity == obj2.productArity =>
obj1.productIterator
.zip(obj2.productIterator)
.zip(obj1.getClass.getDeclaredFields.iterator.map(_.getName))
Copy link
Member

Choose a reason for hiding this comment

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

@urbit-pilled it looks like getDeclaredFields is not working with ScalaJS. Unfortunately the ZIO CI only runs ScalaJS tests with 2.13 so this was not caught before the release but this is showing up in other repos like https://github.com/zio/zio-query/actions/runs/9015988096/job/24771683500?pr=484

Any idea for a workaround?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for the late response, I've been offline this week. Looks like #8841 fixes the ScalaJS and Scala Native issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants