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

Enable our own CI #6547

Open
rhuanjl opened this issue Dec 24, 2020 · 21 comments
Open

Enable our own CI #6547

rhuanjl opened this issue Dec 24, 2020 · 21 comments
Assignees

Comments

@rhuanjl
Copy link
Collaborator

rhuanjl commented Dec 24, 2020

Microsoft have transferred the repository into our control. We now need to activate our own CI.

In the first instance this will be largely based on the check in CI microsoft was running.

(though we need our own instance of it all as the existing webhooks no longer function)

@ppenzin
Copy link
Member

ppenzin commented Dec 25, 2020

Oh, I managed to miss the moment of transition, will do as soon as I get to my laptop.

@ppenzin
Copy link
Member

ppenzin commented Dec 26, 2020

I started, but ended up with two commits to master before opening a PR to batch changes up.

Basic build:

  • Windows
    • Debug
    • Test
    • Release
  • Linux
    • Use libicu-dev
      • Debug
      • RelWithDebInfo (aka Test)
      • Release
  • OSX
    • With icu4c
      • Debug - static library mode
      • RelWithDebInfo (aka Test)
      • Release
      • Debug with Jit disabled - static library mode

Important miscellanea:

  • Style
    • ASCII check
    • EOL check
  • Copyright check
  • Remove obsolete checks from PR pages

Extra credit:

  • Cache build artifacts

@rhuanjl, feel free to add anything.

Edit: I might have enabled Linux Debug CI in #6554, but running tests takes a while, will come back and check on it early tomorrow.

@rhuanjl
Copy link
Collaborator Author

rhuanjl commented Dec 26, 2020

A question is whether we want to target different versions of windows: (7 8 and 10 were what the old CI did) also processors.

Currently Linux and macOS builds only work with x64 BUT windows can be done with x86 and ARM as well - we probably want to keep testing those?

Also could/should things like style checks be done with github actions or the like instead of using Azure for everything? (I don't know what limits/caps our azure provision may have?)

@rhuanjl
Copy link
Collaborator Author

rhuanjl commented Dec 26, 2020

On the cleanup bit the triggers that are still firing appear to be in the github settings for this repository.

Also there's the now defunct jenkins folder in the source tree - though some of the information in those scripts may be useful.

@ppenzin
Copy link
Member

ppenzin commented Dec 26, 2020

On the cleanup bit the triggers that are still firing appear to be in the github settings for this repository.

I think I figured out how disable old checks. There are also numerous webhooks configured - we might want to check if they are still needed.

Also there's the now defunct jenkins folder in the source tree - though some of the information in those scripts may be useful.

Yep, we need to review its contents and reorganize if we want to keep them.

A question is whether we want to target different versions of windows: (7 8 and 10 were what the old CI did) also processors.

Let's see what Azure has in store, we can try older versions if they are available.

Currently Linux and macOS builds only work with x64 BUT windows can be done with x86 and ARM as well - we probably want to keep testing those?

Agree. Did we have an explicit Arm job? It would be good to try anyway.

Also could/should things like style checks be done with github actions or the like instead of using Azure for everything? (I don't know what limits/caps our azure provision may have?)

I think they can be, Azure Pipelines set up is getting more complicated, and keeping simple checks out of it would help. However, we are not yet at the limit - they allow 10 concurrent jobs, with run time up to 6 hours.

@rhuanjl
Copy link
Collaborator Author

rhuanjl commented Dec 26, 2020

I just looked through the old CI, here's the set of builds that were done for every checkin, each heading showed as one check on github but the bullets under it were separate builds that had to be completed before it reported as done, in addition to these there were the style checks and the CLA agreement check.

Windows 7

  • x64 Debug; build + run tests

Windows 8

  • x64 Debug; build + run tests
  • arm Debug; build + run tests
  • arm Test (RelWithDebugINfo); build + run tests
  • arm Release; build only

Windows 10

  • x64 Debug; build + run tests
  • x64 Test (RelWithDebugInfo); build + run tests
  • x64 Release; build only
  • x64 codecoverage - build with "codecoverage" flag - unsure what this does, didn't seem to involve any tests
  • x86 Debug; build + run tests
  • x86 Test (RelWithDebugInfo); build + run tests
  • x86 Release; build only
  • x64 Debug Lite variant; build + run tests ("Lite" is a build time flag that omits the JIT and a few over components to make CC smaller)
  • x64 Debug Disable Jit variant; build + run tests ("Disable Jit" is a build time flag to omit the Jit from CC but not anything else, this is slightly bigger than a "Lite" build)
  • x86 Debug Lite variant, build + run tests
  • x86 Debug Disable Jit variant, build + run tests

macOS

  • static Debug: build + run tests (statically linked library)
  • static Test (RelWithDebugInfo): build + run tests
  • static Releast; build only
  • shared Disable Jit variant; build + run tests (build a shared library but with the disable jit flag)

Linux

  • static Debug; build + run tests
  • static test (RelWithDebugInfo); build + run tests
  • static Release; build only
  • shared Debug; build + run tests
  • shared test (RelWithDebugInfo); build + run tests
  • shared Release; build only
  • static no icu Debug; build + run tests (no icu flag to build without ICU)
  • shared disable jit Test (RelWithDebugInfo); build + run tests

@ppenzin
Copy link
Member

ppenzin commented Dec 27, 2020

OK, I am going to try to follow what the old CI did where possible.

Code coverage should be an instrumented test run, which would show how much of the code base is 'hit' by tests. May be that was disabled at some point. We had a request for that recently.

@rhuanjl do you know what 'disable JIT' builds are for? There are also jobs marked "slow" (despite finishing quite fast) in the old CI, do you know what are those?

Also, to speed up the builds it would be great to cache build files and lunch multiple test jobs at once, since testing seems to be the slowest part of the build.

ppenzin added a commit that referenced this issue Dec 28, 2020
Single target testing

Enable building and testing via a single target (check) - require chakracore
shared library to be built before the test run. Use that to run tests for debug
builds and only build in release mode.

Expand CI matrix to include OSX. Set timeout to 2 hours and make OSX debug
build static (to work around #5876).

For #6547
@rhuanjl
Copy link
Collaborator Author

rhuanjl commented Dec 28, 2020

@rhuanjl do you know what 'disable JIT' builds are for? There are also jobs marked "slow" (despite finishing quite fast) in the old CI, do you know what are those?

"Disable JIT" is a build time flag to remove the JIT from CC and build it to run with the interpreter only - it works and supports all features though performance will drop, it's a smaller binary and needs less memory though. There's also the "Lite build" flag which does disable JIT and a few other features are removed too for an even smaller binary.

"Slow" tests are a few specific tests in the test suite that are expected to take a long time (more than a minute each perhaps), these are omitted from normal test runs, I didn't think the old CI ran these at all - I thought there were builds for them but they were auto skipped hence the rapid finish - they didn't actually do anything.

ppenzin added a commit that referenced this issue Dec 28, 2020
Add MSVC CI jobs, testing on latest Windows VM image using provided build and test wrappers.

For #6547
ppenzin added a commit that referenced this issue Dec 28, 2020
Add MSVC CI jobs, testing on latest Windows VM image using build and
test wrappers. Assign 6 jobs to 'nix CI (all it needs) and the remaining
4 - to MCVS, as it is much faster.

For #6547
ppenzin added a commit that referenced this issue Dec 28, 2020
Use the pattern in Windows CI to create two separate steps for build and
test.

For #6547
@ppenzin
Copy link
Member

ppenzin commented Dec 28, 2020

We could make some of the really fast builds be the prerequisite for longer ones, especially if we are crossing 10-job threshold of the free tier anyway. This way we can bail out early when thigs are obviously broken.

It appears that Microsoft does not provide Arm, Win7 or Win8 agents in Azure, at least for free:

https://docs.microsoft.com/en-us/azure/devops/pipelines/agents/hosted?view=azure-devops&tabs=yaml

Also, it does not look like Arm scripts were published, even for Windows.

@rhuanjl
Copy link
Collaborator Author

rhuanjl commented Dec 28, 2020

We could make some of the really fast builds be the prerequisite for longer ones, especially if we are crossing 10-job threshold of the free tier anyway. This way we can bail out early when thigs are obviously broken.

That's a good idea.

It appears that Microsoft does not provide Arm, Win7 or Win8 agents in Azure, at least for free:

https://docs.microsoft.com/en-us/azure/devops/pipelines/agents/hosted?view=azure-devops&tabs=yaml

Also, it does not look like Arm scripts were published, even for Windows.

Well this is unfortunate - we may have to see if we can find our own arm and legacy testing machines or find a different service for them - I suppose we're unlikely to break anything for arm or legacy systems in editing high level things but if we get into anything low level we'll want that testing set up.

@rhuanjl
Copy link
Collaborator Author

rhuanjl commented Dec 29, 2020

A couple of other things I noticed whilst looking at the old way of running the CI, on macOS and Linux the runtests.sh script which was used to call runtests.py does 2 things we're not currently doing:

  1. It runs a basic hello world test on release builds (see if ch can execute a JS script that prints "Hello World" and "Pass". test/Basics/hello.js)
  2. It runs native tests these are in test/native-tests and were executed via test_native.sh I think these attempt to build some small sample programmes and link them to ChakraCore, though I'm not 100%

@ppenzin
Copy link
Member

ppenzin commented Dec 30, 2020

Well this is unfortunate - we may have to see if we can find our own arm and legacy testing machines or find a different service for them - I suppose we're unlikely to break anything for arm or legacy systems in editing high level things but if we get into anything low level we'll want that testing set up.

Azure definitely has all of the flavors we would need, but I haven't looked into what those might cost. A different approach would be using our own devices with Azure agents or some other form of CI - this way the cost is not in money, but in machine time.

A couple of other things I noticed whilst looking at the old way of running the CI, on macOS and Linux the runtests.sh script which was used to call runtests.py does 2 things we're not currently doing:

  1. It runs a basic hello world test on release builds (see if ch can execute a JS script that prints "Hello World" and "Pass". test/Basics/hello.js)
  2. It runs native tests these are in test/native-tests and were executed via test_native.sh I think these attempt to build some small sample programmes and link them to ChakraCore, though I'm not 100%

That is a big oops on my part, I thought runtests.py was all we needed. I can fix that relatively easily by adding this to check target.

@rhuanjl
Copy link
Collaborator Author

rhuanjl commented Dec 30, 2020

Another note on cleanup there's the netci.groovy file in the root source directory - this was the config file for the old jenkins CI that microsoft retired 2 years ago, I'm not sure if there's anything useful in it - I don't know the "groovy" language, should we just delete it?

@ppenzin
Copy link
Member

ppenzin commented Dec 30, 2020

Follow up on #6559 - there is exclude_jenkins tag which is used by runtests.sh. I think we need to wire in test excludes and "show passes" via CMake, and at the very least set exclude_jenkins in CI.

I've gone the 'native tests'. For every test there is a JavaScript file generating a makefile. At first it makes your eyes bleed, but it is probably the "most portable" solution - as some builds use MSVC and others - CMake. I will add building those to CMake, we will have full runtests.sh workflow automated.

@ppenzin
Copy link
Member

ppenzin commented Dec 30, 2020

Another note on cleanup there's the netci.groovy file in the root source directory - this was the config file for the old jenkins CI that microsoft retired 2 years ago, I'm not sure if there's anything useful in it - I don't know the "groovy" language, should we just delete it?

I think it is safe to delete, it does not look like there is any special knowledge in it.

@rhuanjl
Copy link
Collaborator Author

rhuanjl commented Dec 30, 2020

I had a quick look at the exclude_jenkins tag two things I noted:

  • everything marked with it is also marked with slow so is being excluded anyway with our current builds
  • all of the tests marked with it have a comment saying they're currently Time Zone dependent and should be fixed to not be time zone dependent then the tag should be removed

Maybe leave it for now - no need to wire it through - as the slow tag is covering it - but perhaps add an action to #6445 to remove the Time Zone dependency so these tests can be enabled in the future.

@Penguinwizzard
Copy link
Contributor

You may want to look into enabling ASAN/UBSAN versions of some tests on Linux; they can help to identify some classes of errors, and there's support for ASAN's memory tracking in the Chakra GC.

@rhuanjl
Copy link
Collaborator Author

rhuanjl commented Jan 6, 2021

Another note for TODOs @pleath reminded us in #5704 that some changes (e.g. low level stuff and ones with pervasive impacts like type system changes) really should go through Fuzzing - we talked a while ago at getting OSS-FUZZ set up but never did it - we may want to look at other (free) fuzzing options too.

@rhuanjl
Copy link
Collaborator Author

rhuanjl commented Jan 16, 2021

I've ticked off the style checks above.

We still need to think about Fuzzing AND potentially the Address and Undefined behaviour sanitiser (ASAN & UBSAN) versions as @Penguinwizzard mentioned above.

@rhuanjl
Copy link
Collaborator Author

rhuanjl commented Jan 24, 2021

Also Just thought of something - we really should be testing the noJit variant (compile time flag to build without the backend at all, interpretor only but otherwise fully functional).

I think perhaps add in an x86 windows nojit built AND a macOS static noJit build - this pair would cover noJit on both x86 and x64 and with/without PAL - I know there's some combinations missing there but wanting to balance coverage with speed there. What do you think @ppenzin ?

EDIT: I've added a noJit macOS build in #6583 to ensure that noJIT bytecode is tested

@rhuanjl
Copy link
Collaborator Author

rhuanjl commented Feb 12, 2021

Looking for options for ARM CI, I found shippable: shippable.com which offered free ARM CI but it's being closed down and will be offline in a couple of months. They were bought by https://jfrog.com/ but it's unclear what architectures jfrog offers - their site it a whole lot less clear than shippable's site.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants