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

WIP Integrate with munit v1.0.0 #208

Closed
wants to merge 2 commits into from
Closed

Conversation

valencik
Copy link
Member

@valencik valencik commented Apr 9, 2022

This PR is a work in progress to update to munit v1 which should be out "real soon now".

Test are currently failing because of a breaking change in munit v1.

From the munit v1.0.0-M1 release notes:

The FunSuite.{afterEach,afterAll} methods are now evaluated before custom fixtures instead of after custom fixtures. The ordering for beforeAll and beforeEach is unchanged. The diff below demonstrates an example, assuming you have a fixture named myFixture and a test suite named MySuite. In other words, FunSuite.{before*,after*} methods are now evaluated as a regular Fixture[T] that's always the first element of munitFixtures: Seq[Fixture[_]].

The particular failure test is CatsEffectFixturesPlatformSpec as it relies the old behaviour where the fixture's afterAll was called before the suite's afterAll.

With the new behaviour, the fixture's afterAll is the last thing called.
It's not immediately clear to me how we'd adapt this test to the new behaviour.

Comment on lines +57 to +58
assertEquals(released, 0) // not yet released
released += 1
Copy link
Member

Choose a reason for hiding this comment

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

@valencik so I wonder if this is actually a (documentation) bug in munit 🤔 because the scaladoc for afterAll() here specifically says:

/** Runs once after all test cases and after all suite-local fixtures have been tear down. */

Copy link
Member

Choose a reason for hiding this comment

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

The particular failure test is CatsEffectFixturesPlatformSpec as it relies the old behaviour where the fixture's afterAll was called before the suite's afterAll.

Ah, I'm so sorry 😂 I obviously didn't read your PR description.

@armanbilge
Copy link
Member

It's not immediately clear to me how we'd adapt this test to the new behaviour.

Is this an intended change in MUnit? Why?

@armanbilge
Copy link
Member

Once again I failed basic reading test :) this breaking change is deliberate and covered in the release notes.

@valencik
Copy link
Member Author

Closing in favour of #223

@valencik valencik closed this Jun 25, 2022
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.

2 participants