-
Notifications
You must be signed in to change notification settings - Fork 5
Adding QA jobs and expanding test jobs for CI/CD. #292
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
Conversation
905e86b to
0f1f871
Compare
|
This is ready to merge @CMCDragonkai |
|
MatrixAI/TypeScript-Demo-Lib#35 merged into TS Demo Lib. Changes to be ported here:
|
|
Take note of the changes here: https://github.com/MatrixAI/TypeScript-Demo-Lib/pull/35/files There's quite a few things that I've talked about in the PR that require required as well. Don't forget to fix up the |
|
I noticed that the quality jobs still seem to run even if In the future it maybe better to instead put all the jobs into a DAG instead, then we won't need to worry about this. |
|
Final changes applied here: MatrixAI/TypeScript-Demo-Lib@c393b7d also should be ported here. |
using the We can switch back to using |
|
Yes, but that's why I had to add the |
0fced5d to
48ac3e1
Compare
|
This is ready to be merged after rebasing on the pending gitlab MR. |
48ac3e1 to
84e4909
Compare
|
You can use poll function instead to start and stop.
See https://unix.stackexchange.com/questions/563886/poll-for-the-right-output-of-a-command
Let's not put weird options here.
Alternative our PK agent stop command can use the status file and see that if it is starting it should queue up a stop operation. That would be an interesting exercise. Once you have fixed up status to the new API this should be easier.
Make sure to use a tagged union for the states though.
On 25 November 2021 10:51:01 am AEDT, Brian Botha ***@***.***> wrote:
***@***.*** commented on this pull request.
> @@ -15,6 +15,7 @@ const start = binUtils.createCommand('start', {
passwordFile: true,
});
start.option('-b, --background', 'Starts the agent as a background process');
+start.option('-t --test', 'starts and stops the agent as a quick sanity test.');
I can change it so we can only set it with an env to make it semi-hidden.
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#292 (comment)
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
|
|
Then use fs.watch in the node fs API. Add it to our FileSystem API.
On 25 November 2021 10:51:01 am AEDT, Brian Botha ***@***.***> wrote:
***@***.*** commented on this pull request.
> @@ -15,6 +15,7 @@ const start = binUtils.createCommand('start', {
passwordFile: true,
});
start.option('-b, --background', 'Starts the agent as a background process');
+start.option('-t --test', 'starts and stops the agent as a quick sanity test.');
I can change it so we can only set it with an env to make it semi-hidden.
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#292 (comment)
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
|
|
Actually fs.promises.watch is even better. |
84e4909 to
56f204c
Compare
|
Simplified the tests to just getting the help page for now. If we need something more complex we can come up with something later. |
|
Once #283 has been merged then this can be merged after re-enabling the test jobs. |
4e56ab9 to
0df7463
Compare
|
Jest caching is also incorporated into the CI/CD too now, so hopefully that reduces the amount of compilation time when running the jobs. |
|
I'm seeing that the Anyway, this seemed to work locally, but gitlab CI/CD still has a problem. In other news, still the |
|
Trying to use |
|
Further tests show that randomjob:
image: registry.gitlab.com/matrixai/engineering/maintenance/gitlab-runner
stage: test
script:
- echo "$TS_CACHED_TRANSPILE_CACHE"
- echo "$TS_CACHED_TRANSPILE_PORTABLE"
- echo "$CI_PROJECT_DIR"
- echo $(pwd)
- echo doneHowever, if used inside However this is only a problem when used in the child pipeline. The parent pipeline is fine with using This also means setting |
|
I had to submit a support request for this: https://support.gitlab.com/hc/en-us/requests/260663 In the mean time, I'll have to use job-specific variables to use |
|
Seems like child pipeline variables has some problems. Furthermore the top-level variables acts like the "default" variables in each job section. That's probably good to know, so you can't refer to top level variables when creating job-level variables. |
|
Ok I have found out what the problem is.
|
|
Finally with the ts-node-cache applied foreground starts took about 9 seconds. This is about more than 2x the time it takes to run on my own computer here, so the CI/CD computers are not as powerful. Without the ts-node cache, it takes about 22 seconds, which is double the time. Anyway the per-test default timeout is 20s, and we x2 when starting pk agents raw, so this should be sufficient. |
…om the parent pipeline
67040df to
74bec18
Compare
|
Ok some tests are expected to fail. Regardless, we are going to now test the final builds and QA runs and then if these all work, then we should be able to pass and merge. |
|
The |
|
There's a This is something we can debug locally. Failing on nix builds: |
|
This is really quite strange error. It works fine on TypeScript-Demo-Lib, but something changed in the middle. Adding And there are thousands of debug lines above. @tegefaulkes any ideas why this is happening? Note that in Ts demo lib, the debug messages start to say |
|
It appears to work once I've added the |
|
It turns out that the license switch to GPL3 is what caused it to fail. When setting |
|
The list of licenses that PKG considers to be foss is here https://github.com/vercel/pkg/blob/main/lib/walker.ts#L118-L142 It has: But not |
|
Solution is to add |
|
Builds work and pipeline passed. Time to clean up the scaffolding and merge!!! https://gitlab.com/MatrixAI/open-source/js-polykey/-/pipelines/444133744 |
608e387 to
760ae54
Compare
|
If I remember correctly some modules are included as bytecode for license reasons. You can disable that by using a certain argument but I forget what it is exactly. |
|
All done. @emmacasolin @tegefaulkes please review the pipelines page to see what the test failures are as you are working on your PRs: |

Description
This PR updates the gitlab CI/CD with new jobs for testing if the nix-build outputs run.
Note the pinning of
node-gyp-builddependency discussed here: MatrixAI/TypeScript-Demo-Lib#35 (comment).Native dependencies are flaky and has to be considered specially when upgrading.
Issues Fixed
ConnectionReversewarning:[ERR_STREAM_WRITE_AFTER_END]: write after end#300network/Connection.ts#293Tasks
.gitlab-ci.ymlis that ts-node cache is reused. Session Bin Test Fixes #296 (comment)testBinUtils.pkAgentwithtestUtils.setupGlobalAgenttestUtils.setupGlobalKeyPairis sufficient, in which case no need to share a global agent, it will be faster[ ] Add back in worker manager related testing- KeyManager already has tests for worker manager, CLI cannot test worker manager due to weird threadsjs behaviour when spawned as child process inside jest, and the last would just be DB functions which is already tested in js-dbpk agent start, by using a--coresparameter[ ] Test the- cannot use workers in--workersparameter, by default it's set to all cores, need to ensure that we are using just 1 worker for testingpkSpawnthus bin tests are not able to test it at allallowHalfOpenisn't working in utp-native - TheallowHalfOpendoesn't seem to be working on the server mafintosh/utp-native#41 (comment)PolykeyAgent,vaultsandnodes.gitlab-ci.ymlfile not to have the special case for this branch as it will be merged into master.node-gyp-builddependencyFinal checklist