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

Add support for record puns #1710

Merged
merged 4 commits into from
Mar 23, 2020
Merged

Add support for record puns #1710

merged 4 commits into from
Mar 23, 2020

Conversation

Gabriella439
Copy link
Collaborator

... as standardized in dhall-lang/dhall-lang#938

@Gabriella439 Gabriella439 mentioned this pull request Mar 20, 2020
2 tasks
@sjakobi
Copy link
Collaborator

sjakobi commented Mar 20, 2020

I've pushed a patch that should fix nix-build.

@@ -182,7 +182,6 @@ Extra-Source-Files:
dhall-lang/tests/import/cache/dhall/1220efc43103e49b56c5bf089db8e0365bbfc455b8a2f0dc6ee5727a3586f85969fd
dhall-lang/tests/import/data/*.dhall
dhall-lang/tests/import/data/*.txt
dhall-lang/tests/import/data/fieldOrder/*.dhall
dhall-lang/tests/import/failure/*.dhall
dhall-lang/tests/import/success/*.dhall
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think these glob entries wouldn't include test files like tests/import/success/unit/AlternativeChain1A.dhall. We might be missing a bunch of test cases then, at least when running them with nix.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@sjakobi: Yeah, we are likely missing a bunch of test cases, especially after the refactor. I believe newer versions of Cabal support ** syntax for globbing through nested directories, so then we would be able to write: dhall-lang/tests/**.dhall

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, the syntax is a bit more complicated and inconvient though: https://cabal.readthedocs.io/en/latest/file-format-changelog.html#cabal-version-2-4

I hope requiring Cabal-2.4 is ok. I think it would make building with Nix and GHC < 8.6 more tricky though.

In any case, I think we should address this issue separately.

@sjakobi
Copy link
Collaborator

sjakobi commented Mar 21, 2020

Hydra reports two test failures in https://hydra.dhall-lang.org/build/54229/nixlog/5:

./dhall-lang/tests/type-inference/success/CacheImportsCanonicalize:
Testing without real HTTP support -- using mock HTTP client to resolve remote import.
FAIL
        tests/Dhall/Test/TypeInference.hs:64:
        user error ((mock http) Url does not match any of the hard-coded rules: https://csrng.net/csrng/csrng.php?min=0&max=1000)
./dhall-lang/tests/type-inference/success/CacheImports:
Testing without real HTTP support -- using mock HTTP client to resolve remote import.
FAIL
        tests/Dhall/Test/TypeInference.hs:64:
        user error ((mock http) Url does not match any of the hard-coded rules: https://csrng.net/csrng/csrng.php?min=0&max=1000)

@sjakobi
Copy link
Collaborator

sjakobi commented Mar 21, 2020

I'm confused why such tests would live in the type-inference directory…

@sjakobi
Copy link
Collaborator

sjakobi commented Mar 21, 2020

My last commit should handle the two CacheImports test failures. When running the tests locally I'm seeing a different issue though:

$ cabal test dhall:tasty -w ghc-8.6.5 --test-show-details=streaming --test-options "--hide-successes"
...
Test suite tasty: RUNNING...
Dhall Tests
  import tests
    discover
      ./dhall-lang/tests/import/success/unit/asLocation/HashA.dhall:                                                           PASS (unexpected)
        (unexpected success)

1 out of 1560 tests failed (10.75s)
...

@sjakobi
Copy link
Collaborator

sjakobi commented Mar 22, 2020

I think I might have had some old state in my dhall/.cache affecting the results. After cleaning it, all tests behave as expected, both with cabal and stack.

@sjakobi
Copy link
Collaborator

sjakobi commented Mar 22, 2020

I think the Travis failure with

An error occurred while generating the build script.

…is probably safe to ignore.

@Gabriella439
Copy link
Collaborator Author

@sjakobi: Yeah, I think it was a temporary Travis outage. I'll rerun it

@Gabriella439 Gabriella439 merged commit e0c0faf into master Mar 23, 2020
@Gabriella439 Gabriella439 deleted the gabriel/record_puns branch March 23, 2020 01:56
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.

None yet

2 participants