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

nim CI doesn't actually test this package #80

Closed
timotheecour opened this issue Mar 15, 2020 · 3 comments
Closed

nim CI doesn't actually test this package #80

timotheecour opened this issue Mar 15, 2020 · 3 comments

Comments

@timotheecour
Copy link
Contributor

same issue as tulayang/asyncmysql#5
either a test task needs to be defined or a tests folder needs to exist

@flyx
Copy link
Owner

flyx commented Mar 15, 2020

Please give more information, I don't really understand the problem. Are you talking about this service which currently 502s? What is it? How is it relevant to NimYAML?

Also, there is a test task, nim tests. While the currently configured CI doesn't work for reasons that escape me, I don't actually need it since NimYAML is in maintenance mode and for the rare changes I make, executing the tests manually works well enough.

@timotheecour
Copy link
Contributor Author

timotheecour commented Mar 15, 2020

Please give more information

when nim CI runs, it tests select packages to avoid introducing regressions in nim pacakges when new commits are added to nim, including NimYAML pacakage, see https://github.com/nim-lang/Nim/blob/9eeb514dda08f1caadb0d8e01a8595d991530b52/testament/important_packages.nim#L95

pkg "yaml"

the default action is cmd = "nimble test" (see https://github.com/nim-lang/Nim/blob/9eeb514dda08f1caadb0d8e01a8595d991530b52/testament/important_packages.nim#L2)

this all seemed to work, giving false impression that NimYAML was being tested in nim CI, whereas in fact nim-lang/nimble#780 reveals this was not the case.

The simplest fix is to add a task test to this package (not nim tests), or we'll have to disable testing for NimYAML in nim CI.

Also, there is a test task, nim tests. While the currently configured CI doesn't work for reasons that escape me

that doesn't help as you can see by running the default test command (being run in nim CI) nimble test.

see https://github.com/nim-lang/nimble#tests for what's supported (as other packages do):
either a test task in yaml.nimble, or have a tests dir (not test) and make sure they're actually being tested when running nimble test

I don't actually need it since NimYAML is in maintenance mode and for the rare changes I make, executing the tests manually works well enough

that's not the only reason for testing in nim CI, the other important reason is to make sure new PR's in nim repo don't break nim ecosystem (including NimYAML, on which other nimble packages depend on), as explained in nim-lang/Nim#8638

@flyx
Copy link
Owner

flyx commented Mar 22, 2020

Thanks for the explanation, fixed it.

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

No branches or pull requests

2 participants