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

testsuite: Refactor withShorterPathForNewBuildStore #9505

Merged
merged 1 commit into from
Apr 19, 2024

Conversation

mpickering
Copy link
Collaborator

This makes withShorterPathForNewBuildStore fit more nicely into the rest of the testing infrastructure.

  • Move withShorterPathForNewBuildStore to TestM monad
  • Move responsibility for passing --store-dir to cabalGArgs function
  • Move findDependencyInStore into TestM, and remove requirement to pass path to store directory.
  • Introduce testStoreDir function which returns the store location (and honours withShorterPathForNewBuildStore)
  • Migrate tests which use withShorterPathForNewBuildStore.

Please read Github PR Conventions and then fill in one of these two templates.

@mpickering mpickering force-pushed the wip/refactor-testsuite branch 2 times, most recently from 8ff6d70 to fb56227 Compare December 8, 2023 17:24
@mpickering
Copy link
Collaborator Author

Can someone review this please?

Copy link
Member

@Kleidukos Kleidukos left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@Mikolaj Mikolaj left a comment

Choose a reason for hiding this comment

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

Thank you.

@Mikolaj
Copy link
Member

Mikolaj commented Dec 21, 2023

@mpickering: ready for the label!

@mpickering mpickering added merge me Tell Mergify Bot to merge and removed merge me Tell Mergify Bot to merge labels Jan 19, 2024
@mpickering
Copy link
Collaborator Author

This needs rebasing as some tests are updated to use the old API.

@mpickering mpickering force-pushed the wip/refactor-testsuite branch from fb56227 to f78b707 Compare February 9, 2024 12:04
@alt-romes alt-romes force-pushed the wip/refactor-testsuite branch from f78b707 to c906452 Compare March 7, 2024 14:53
@alt-romes
Copy link
Collaborator

I've rebased this commit (basically cherry-picked from the private dependencies branch where it was already updated)

@alt-romes alt-romes added the merge me Tell Mergify Bot to merge label Mar 7, 2024
@alt-romes alt-romes force-pushed the wip/refactor-testsuite branch 3 times, most recently from 0e6593c to ebbd10f Compare March 7, 2024 17:24
@alt-romes alt-romes removed the merge me Tell Mergify Bot to merge label Mar 8, 2024
@alt-romes alt-romes force-pushed the wip/refactor-testsuite branch 3 times, most recently from 0932a33 to 555e791 Compare March 8, 2024 16:24
@alt-romes alt-romes added the merge me Tell Mergify Bot to merge label Mar 11, 2024
@Mikolaj
Copy link
Member

Mikolaj commented Apr 16, 2024

@alt-romes: I'm afraid you've lost the race with mergify (and possibly other PR authors). Could you resolve the conflicts one last time and I'd try to prod mergify until it yields?

This makes `withShorterPathForNewBuildStore` fit more nicely into the
rest of the testing infrastructure.

* Move `withShorterPathForNewBuildStore` to `TestM` monad
* Move responsibility for passing `--store-dir` to `cabalGArgs` function
* Move `findDependencyInStore` into `TestM`, and remove requirement to
  pass path to store directory.
* Introduce `testStoreDir` function which returns the store location
  (and honours `withShorterPathForNewBuildStore`)
* Migrate tests which use `withShorterPathForNewBuildStore`.
@alt-romes alt-romes force-pushed the wip/refactor-testsuite branch from 555e791 to 2a2d0b3 Compare April 19, 2024 10:03
@Mikolaj
Copy link
Member

Mikolaj commented Apr 19, 2024

@alt-romes: thanks a lot! The merge label is already there and since the PR has been blocked on a merge conflict for so long, I think we can manually set merge_delay_passed to make sure no more conflicts emerge, unless there are any objections before CI finishes.

@Mikolaj Mikolaj added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Apr 19, 2024
@Mikolaj
Copy link
Member

Mikolaj commented Apr 19, 2024

@mergify regresh

Copy link
Contributor

mergify bot commented Apr 19, 2024

regresh

❌ Sorry but I didn't understand the command. Please consult the commands documentation 📚.

@Mikolaj
Copy link
Member

Mikolaj commented Apr 19, 2024

@mergify refresh

Copy link
Contributor

mergify bot commented Apr 19, 2024

refresh

✅ Pull request refreshed

@Mikolaj
Copy link
Member

Mikolaj commented Apr 19, 2024

@mergify queue

Copy link
Contributor

mergify bot commented Apr 19, 2024

queue

🛑 This queue command has been cancelled by a dequeue command

@Mikolaj
Copy link
Member

Mikolaj commented Apr 19, 2024

Mergify got confused. It seems to think that -mergify-configuration-changed and that there's not enough reviews. I may end up merging manually.

@Mikolaj
Copy link
Member

Mikolaj commented Apr 19, 2024

@mergify unqueue

Copy link
Contributor

mergify bot commented Apr 19, 2024

unqueue

✅ The pull request is not waiting to be queued anymore.

@Mikolaj
Copy link
Member

Mikolaj commented Apr 19, 2024

@mergify queue

Copy link
Contributor

mergify bot commented Apr 19, 2024

queue

🛑 The pull request has been merged manually

The pull request has been merged manually at 774ff04

@Mikolaj
Copy link
Member

Mikolaj commented Apr 19, 2024

@mergify rebase

Copy link
Contributor

mergify bot commented Apr 19, 2024

rebase

☑️ Nothing to do

  • any of:
    • #commits-behind>0 [📌 rebase requirement]
    • #commits>1 [📌 rebase requirement]
    • -linear-history [📌 rebase requirement]
  • -closed [📌 rebase requirement]
  • -conflict [📌 rebase requirement]
  • queue-position=-1 [📌 rebase requirement]

@Mikolaj Mikolaj merged commit 774ff04 into haskell:master Apr 19, 2024
47 checks passed
@ulysses4ever
Copy link
Collaborator

@mergify backport 3.12

Copy link
Contributor

mergify bot commented May 29, 2024

backport 3.12

✅ Backports have been created

mergify bot added a commit that referenced this pull request May 30, 2024
…10059)

* testsuite: Refactor withShorterPathForNewBuildStore

This makes `withShorterPathForNewBuildStore` fit more nicely into the
rest of the testing infrastructure.

* Move `withShorterPathForNewBuildStore` to `TestM` monad
* Move responsibility for passing `--store-dir` to `cabalGArgs` function
* Move `findDependencyInStore` into `TestM`, and remove requirement to
  pass path to store directory.
* Introduce `testStoreDir` function which returns the store location
  (and honours `withShorterPathForNewBuildStore`)
* Migrate tests which use `withShorterPathForNewBuildStore`.

(cherry picked from commit 2a2d0b3)

# Conflicts:
#	Cabal-tests/Cabal-tests.cabal
#	Cabal-tests/lib/Test/Utils/TempTestDir.hs

* !fixup resolve conflicts

* fixup! always import `(</>)`

---------

Co-authored-by: Matthew Pickering <[email protected]>
Co-authored-by: Artem Pelenitsyn <[email protected]>
Co-authored-by: brandon s allbery kf8nh <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days merge me Tell Mergify Bot to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants