Skip to content

GitHub actions #7035

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

Merged
merged 48 commits into from
Dec 3, 2020
Merged

GitHub actions #7035

merged 48 commits into from
Dec 3, 2020

Conversation

davimacedo
Copy link
Member

No description provided.

@Moumouls
Copy link
Member

Moumouls commented Dec 3, 2020

you're right. I will go to the 18.04

I saw that you were fooled by the version selector like me!

@davimacedo
Copy link
Member Author

Yes.

Tests for 4.4.2 failed. Maybe we should address this issue on a separate pr. I will downgrade to 4.0.21.

@davimacedo
Copy link
Member Author

Now we just need to pray for the flaky tests do not fail :)

@Moumouls
Copy link
Member

Moumouls commented Dec 3, 2020

@davimacedo mongo version is correct i think, npm run lint seems to cancel the workflow don't know why

@davimacedo
Copy link
Member Author

@davimacedo
Copy link
Member Author

I will revert back to ubuntu 16

@Moumouls
Copy link
Member

Moumouls commented Dec 3, 2020

I didn't know that a failure in the matrix could cause the other matrix to crash.

@Moumouls
Copy link
Member

Moumouls commented Dec 3, 2020

@davimacedo it's not a problem with ubuntu it's the Node Version, mongodb runner is not usable under Node14/15

@davimacedo
Copy link
Member Author

By default, they cancel the other jobs within the same matrix but there is an option that you change this behavior. I think it is better to actually cancel though, right?

@Moumouls
Copy link
Member

Moumouls commented Dec 3, 2020

pull request was sent but new package version not released yet: mongodb-js/runner#174

@davimacedo
Copy link
Member Author

Makes sense.

@davimacedo
Copy link
Member Author

In this case I will go back to 18.04 and remove the newest node. We put it back once mongodb supports it. ok?

@Moumouls
Copy link
Member

Moumouls commented Dec 3, 2020

@davimacedo
Copy link
Member Author

finally passed through mongodb-runner

@davimacedo
Copy link
Member Author

davimacedo commented Dec 3, 2020

I think we are good now. Maybe wait to see if @dplewis has any additional thoughts and merge?

@Moumouls
Copy link
Member

Moumouls commented Dec 3, 2020

Finally the CI

@Moumouls
Copy link
Member

Moumouls commented Dec 3, 2020

@davimacedo is it possible the reduce the codecov threshold ?

@davimacedo
Copy link
Member Author

It looks it is possible: https://docs.codecov.io/docs/commit-status

Copy link
Member

@dplewis dplewis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@davimacedo davimacedo merged commit 54a61b7 into master Dec 3, 2020
@davimacedo davimacedo deleted the githubActions branch December 3, 2020 16:15
@mtrezza
Copy link
Member

mtrezza commented Feb 3, 2021

@dplewis I took a quick look at this because you mentioned it; looking into the CI logs I see some misconfigs like different MongoDB versions in pretest vs. test, also MongoDB >=4.2 is not compatible with MMAPv1 storage engine, mongo-runner will simply fail to start. Anyway, tests pass now for all recent MongoDB versions in #7161.

branches:
- '**'
env:
COVERAGE_OPTION: ./node_modules/.bin/nyc
Copy link
Member

@mtrezza mtrezza Apr 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davimacedo just wondering, what is COVERAGE_OPTION used for? I haven't found any references.

Edit: Found a ref, I think that can be removed, maybe from travis ci?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it makes this to work.

Copy link
Member

@mtrezza mtrezza Apr 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How so?

I believe it was part of a script in package.json, that has been removed:

cross-env NODE_ENV=test TESTING=1 ./node_modules/.bin/babel-node $COVERAGE_OPTION ./node_modules/jasmine/bin/jasmine.js

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might be right. Let's try to remove it and see what happens.

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

Successfully merging this pull request may close these issues.

4 participants