Skip to content
This repository has been archived by the owner on Jul 23, 2019. It is now read-only.

Ci test script #36

Closed
wants to merge 1 commit into from
Closed

Ci test script #36

wants to merge 1 commit into from

Conversation

lucat1
Copy link

@lucat1 lucat1 commented Mar 11, 2018

This script differs(via git) the files changed from the origin's default branch and executes tests only on changed packages. Since a GIF is worth more than a thousand words, here you have a demonstration:

gif

The code is a little messy and not well commented, but I think it's okay for a first quick sketch. Tell me if you can find a simpler way to build these dependencies trees, I sincerely couldn't.

Also would you mind taking a look at this comment here and give me any suggestions?
https://github.com/LucaT1/xray/blob/f215a82acaafcc4068f124bce6463be013b6a3f1/scripts/test.js#L5-L6

@totsteps
Copy link

Off topic but, @lucat1 I really like your setup and would love to know about your vim editor, dotfiles, theme, plugins etc. Thanks 😃

@lucat1
Copy link
Author

lucat1 commented Mar 18, 2018

These are my oh-my-zsh themes and vim config: https://gist.github.com/LucaT1/5228b631b3a89378b6b78eda70cc71cd

This is what i use for autosuggestions: https://github.com/zsh-users/zsh-autosuggestions

But let's try to keep the conversation around this PR.

@as-cii
Copy link
Contributor

as-cii commented Mar 19, 2018

Thanks for this pull request, @lucat1!

As @nathansobo mentioned on #22, we are in the process of revising the architecture of xray and this may affect the way things get tested. We'll make sure to take a look at this once #46 lands on master. Thanks!

@lucat1
Copy link
Author

lucat1 commented Mar 19, 2018

No problem, take your time ❤️

@nathansobo
Copy link

Hey @lucat1! Thanks for your patience. #46 has now been merged, so do you want to take another look at this based on the new reality on master and ping us when you have something to review?

I'm also curious about the best way to integrate this script with CI. Does Travis provide some sort of environment variable to help indicate the diff being tested. For example, what if the PR is against a different target branch than master?

One thing we just realized is that running tests in this way sort of changes the definition of "green" on a PR. If you open a PR against a branch that currently has a failing build, a build green of that branch doesn't necessarily mean that this PR "fixes" the build. That's probably okay, since we should strive to target branches that have green builds with most PRs. Extra bonus points would be to detect that the target branch has a failing build and revert to running a full build in that scenario.

@lucat1
Copy link
Author

lucat1 commented Mar 22, 2018

I'll start working on this tomorrow, but I'm going to be out this weekend so some commits will probably show up on Monday.

As to answer some of the things you pointed out:

  • I don't think the new architecture changes anything, but if you'd like to run tests also on rust packages I'd love to know what commands I should run.

  • To figure out the branch the PR is against we can take the TRAVIS_PULL_REQUEST env variable and contact the GitHub API to get the necessary pieces of information.

  • Based on the point above with the Travis CI API we can then figure out if the base branch is failing and by consequence.

These are just ideas I came up with by reading some docs, but there may be better options. I'm widely open to suggestions, but for now, I'll start writing some code down and see how it goes.

@lucat1
Copy link
Author

lucat1 commented Mar 25, 2018

@nathansobo just a friendly ping to know how You'd like to proceed.

@lucat1
Copy link
Author

lucat1 commented Apr 3, 2018

After #48 has been merged and the repo's structure has changed, I feel like this PR is irrelevant.

@lucat1 lucat1 closed this Apr 3, 2018
@nathansobo
Copy link

Thanks anyway for trying @lucat1

@lucat1 lucat1 deleted the ci-test-script branch April 3, 2018 18:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants