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

Minimal test suite #81

Merged
merged 13 commits into from
Sep 15, 2023
Merged

Minimal test suite #81

merged 13 commits into from
Sep 15, 2023

Conversation

andreabedini
Copy link
Member

@andreabedini andreabedini commented Sep 11, 2023

Closes: #7

  • Add support for urls with file: schema; both absolute (file:/path) and
    relative (file:path) paths are supported.

  • Log curl invocation in case of failure

  • Rename fetchRemoteAsset to fetchURL

  • Add verbosity flag

  • Bump GHC to 9.4.7

  • Bump flake inputs

Feedback is welcome.

Some design issues:

  • Source tarballs: should we check them in? create them on the fly? for now they are in tests/fixtures/simple/tarballs along with their unpacked sources but it's not a great solution.
  • withFixtures creates a temporary directory and symlinks the fixture files. This allows running the tests in parallel but that temporary gets wiped when the test finish. Maybe it's better to keep it around?

TODO

  • Accept a file:// url that points to a cabal file, foliage will make a source distribution from the package in that folder. It's a good idea but there are too many design decision involved. Leaving this for another time.

- Add support for urls with file: schema; both absolute (file:/path) and
  relative (file:path) paths are supported.

- Log curl invocation in case of failure

- Rename fetchRemoteAsset to fetchURL

- Add verbosity flag

- Bump GHC to 9.4.7

- Bump flake inputs
@andreabedini andreabedini self-assigned this Sep 11, 2023
@andreabedini andreabedini marked this pull request as ready for review September 11, 2023 09:18
@michaelpj
Copy link
Collaborator

Source tarballs: should we check them in? create them on the fly?

I'd be inclined to create them on the fly, although that's more code.

withFixtures creates a temporary directory and symlinks the fixture files. This allows running the tests in parallel but that temporary gets wiped when the test finish. Maybe it's better to keep it around?

I definitely think running them in a tmpdir is a good idea, but yeah, having a flag to keep the directory is super useful if things fail.

app/Foliage/Options.hs Outdated Show resolved Hide resolved
cabal.project Outdated Show resolved Hide resolved
flake.nix Outdated Show resolved Hide resolved
tests/Foliage/Tests/Utils.hs Outdated Show resolved Hide resolved
tests/Foliage/Tests/Utils.hs Outdated Show resolved Hide resolved
tests/Foliage/Tests/Utils.hs Outdated Show resolved Hide resolved
tests/Tests.hs Show resolved Hide resolved
foliage.cabal Show resolved Hide resolved
Copy link
Contributor

@yvan-sraka yvan-sraka left a comment

Choose a reason for hiding this comment

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

LGTM :)

app/Foliage/Options.hs Show resolved Hide resolved
app/Foliage/PrepareSource.hs Outdated Show resolved Hide resolved
app/Foliage/PrepareSource.hs Outdated Show resolved Hide resolved
flake.nix Outdated Show resolved Hide resolved
@andreabedini andreabedini merged commit cbd0c5d into main Sep 15, 2023
3 checks passed
@andreabedini andreabedini deleted the andrea/testsuite branch September 15, 2023 04:18
yvan-sraka pushed a commit to yvan-sraka/foliage that referenced this pull request Sep 22, 2023
* Minimal test suite

- Add support for urls with file: schema; both absolute (file:/path) and
  relative (file:path) paths are supported.

- Log curl invocation in case of failure

- Rename fetchRemoteAsset to fetchURL

- Add verbosity flag

- Bump GHC to 9.4.7

- Bump flake inputs

* Apply suggestions from code review

Co-authored-by: Michael Peyton Jones <me@michaelpj.com>

* Add short option '-v' for '--verbosity'

* Whitespace

* Add comment explaining why the dot

* Rename withFixture to inTemporaryDirectoryWithFixture

* Small refactor of PrepareSource

* Rename TarballSource to URISource

- Move sourceUrl to Foliage.Meta.packageVersionSourceToUri

* Simplify inTemporaryDirectoryWithFixture

* Document tar and cp flags

* Reformat

---------

Co-authored-by: Michael Peyton Jones <me@michaelpj.com>
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.

There's no tests 😞
3 participants