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

should we have defined(appveyor), defined(travis), defined(continousintegration) ? #8515

Closed
timotheecour opened this issue Aug 2, 2018 · 3 comments

Comments

@timotheecour
Copy link
Member

timotheecour commented Aug 2, 2018

should we have defined(appveyor), defined(travis), defined(continousintegration) ?
semantics:

travis sets -d:travis -d:continousintegration
appveyor sets -d:appveyor -d:continousintegration

use case: selectively enabling/disabling tests based on these. Eg, in https://github.com/nim-lang/Nim/pull/8510/files I disabled a test for windows, but what I really wanted was to disable it for appveyor in case one wants to run that test on windows somewhere else (eg locally)

There are other use cases, eg:

  • if some tests are really slow or memory intensive, we could use this logic:
# when locally: always runs; when under continuous integration: only runs 10% of time
when not defined(continousintegration) or rand(1.0) or true < 0.1: run_expensive_sanity_test()

# when locally: always runs; when under appveyor: only runs when no resource contention
when not defined(appveyor) or cpuLoad() < 2: run_expensive_sanity_test()
@cheatfate
Copy link
Member

cheatfate commented Aug 2, 2018

I don't see any reasons why we need this specific defines specially for testing platforms, it will allow some cheating tests which will work in CI but will not work in production.

You just need to improve your tests, obtain amount of available RAM and adjust your allocation behavior, but if amount of available RAM is not enough to complete test, then you can just skip it.

@timotheecour
Copy link
Member Author

you're probably right, it'd be more robust (but requires some APIs). So I guess what's needed is a high-level testing API that wraps common things like:

  • cpuLoad()
  • availableRAM()
    (which could simply be wrappers that call OS-specific cmd line calls to get these for implementation simplicity)

@timotheecour
Copy link
Member Author

timotheecour commented Aug 7, 2018

related: see in $nimc_D/tests/testament/specs.nim :

let isTravis = existsEnv("TRAVIS")
let isAppVeyor = existsEnv("APPVEYOR")

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