chakra-core / ChakraCore

ChakraCore is an open source Javascript engine with a C API.
MIT License
9.12k stars 1.2k forks source link

Enable our own CI #6547

Open rhuanjl opened 3 years ago

rhuanjl commented 3 years ago

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 commented 3 years ago

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

ppenzin commented 3 years ago

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

Basic build:

Important miscellanea:

Extra credit:

@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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

Windows 8

Windows 10

macOS

Linux

ppenzin commented 3 years ago

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.

rhuanjl commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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.