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

added jjbigorra and this maintainers packages (keuringsdienst, yggdrasil-schema) #7518

Merged
merged 2 commits into from
Sep 18, 2024

Conversation

jjba23
Copy link
Contributor

@jjba23 jjba23 commented Sep 18, 2024

Checklist:

  • Meaningful commit message, eg add my-cool-package (please don't mention build-constraints.yml)
  • At least 30 minutes have passed since uploading to Hackage
  • If applicable, required system libraries are added to 02-apt-get-install.sh or 03-custom-install.sh
  • (recommended) Package have been verified to work with the latest nightly snapshot, e.g by running the verify-package script
  • (optional) Package is compatible with the latest version of all dependencies (Run cabal update && cabal outdated)

The script runs virtually the following commands in a clean directory:

  stack unpack $package-$version # `-$version` is optional
  cd $package-$version
  rm -f stack.yaml && stack init --resolver nightly --ignore-subdirs
  stack build --resolver nightly --haddock --test --bench --no-run-benchmarks

Copy link
Contributor

@mihaimaruseac mihaimaruseac 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

@jjba23
Copy link
Contributor Author

jjba23 commented Sep 18, 2024

@mihaimaruseac thank you! I am really happy to finally understand enough Haskell to make my own libraries, and to use Stack 😄 how can we get this PR merged?

@mihaimaruseac
Copy link
Contributor

Sorry, I was waiting for the check job to finish and then I forgot to check it, started doing other things. Will merge it now.

@mihaimaruseac mihaimaruseac merged commit ac32ee6 into commercialhaskell:master Sep 18, 2024
1 check passed
@mihaimaruseac
Copy link
Contributor

There are some test failures so I'll mark yggdrasil-schema as expect-test-failures:

           yggdrasil                     
             can parse sql DDL statements for SQLite [✘]
             can explode migration filename properly [✔]
             gets files in the right order [✘]
             does run migrations correctly on clean DB [✘]
             does skip migrations correctly on used DB [✘]
             does not do anything when runMigrations is false [✘]                                                                                                                                                   
                    
           Failures:
                               
             test/Yggdrasil/Test/Yggdrasil.hs:12:3: 
             1) yggdrasil can parse sql DDL statements for SQLite
                  uncaught exception: IOException of type NoSuchThing
                  ./resources/test/migration-files/1-some-valid-sql.sql: withBinaryFile: does not exist (No such file or directory)
            
             To rerun use: --match "/yggdrasil/can parse sql DDL statements for SQLite/" --seed 566057077
            
             test/Yggdrasil/Test/Yggdrasil.hs:24:3: 
             2) yggdrasil gets files in the right order
                  uncaught exception: IOException of type NoSuchThing
                  ./resources/test/migration-files/: getDirectoryContents:openDirStream: does not exist (No such file or directory)
            
             To rerun use: --match "/yggdrasil/gets files in the right order/" --seed 566057077

...

Please fix tests and re-enable so you'd be notified if a dependency update under you starts breaking tests.

mihaimaruseac added a commit that referenced this pull request Sep 18, 2024
@jjba23
Copy link
Contributor Author

jjba23 commented Sep 18, 2024

I appreciate it @mihaimaruseac ! I believe I fixed the tests in the latest version of yggdrasil-schema, namely 1.0.0.5, since in Github CI also had issues in my own repo, but I believe the stack CI picked the older one if my assumption is correct, how could we check?

@mihaimaruseac
Copy link
Contributor

@jjba23
Copy link
Contributor Author

jjba23 commented Sep 19, 2024

@mihaimaruseac thanks! I am really puzzled then why the tests fail. It must have to do with that the tests in Yggdrasil expect to be run from the root directory of the project, so that ./resources/test/migration-files is available. Where could I check in the Stackage CI how this is done, what is the "present working directory" or even better, what can i do to fix it :)

@mihaimaruseac
Copy link
Contributor

To fix, can you make the test read the file using absolute path based on where the binary is?

@jjba23
Copy link
Contributor Author

jjba23 commented Sep 20, 2024

To fix, can you make the test read the file using absolute path based on where the binary is?

Thanks for the reply. Unfortunately I have no idea of how to do this, or if it even is possible to be honest. Does anybody have some tips? The real test of this tool is to "actually" read files from disk, so I can't think of a better way of doing this. It works perfectly fine in GitHub actions, due to the pwd I suppose.
Also without knowledge or access to how Stack is running these pipelines I am a bit clueless. I'd really appreciate a hand here. @mihaimaruseac

@jjba23
Copy link
Contributor Author

jjba23 commented Sep 20, 2024

It's possible you will also need to flag free-alacarte as failing tests since it also contains a test that reads from a file @mihaimaruseac

@mihaimaruseac
Copy link
Contributor

Oh, I already did that.

For testing with an absolute path, I think https://stackoverflow.com/questions/12361594/how-to-get-directory-of-executable-in-haskell would be helpful. I'm in airport now and cannot test myself :(

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