-
Notifications
You must be signed in to change notification settings - Fork 134
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
partial tests in ci #997
partial tests in ci #997
Conversation
@@ -172,6 +172,6 @@ module.exports = class TestJob { | |||
} | |||
|
|||
get configForLog() { | |||
return `${this.mode.padEnd(" ", 8)} / ${(this.polyfillCombinations ? 'combined' : ' ')} / ${(this.shard ? 'shard ' + this.shard + ' ' : 'shard n/a')}`; | |||
return `${this.mode.padEnd(" ", 8)} / ${(this.polyfillCombinations ? 'combined' : ' ')}${(this.shard ? ' / shard ' + this.shard + ' ' : '')}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only IE8/9 need this, for all other tests it just bloats the logs.
@JakeChampion Also added some unit tests to validate the logic and at this point I am generally happy with the state of this change. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is great!
Changes for tests in CI :
master
.master
contains few changes only inpolyfills/...
.1 : Scheduled job for polyfill combinations.
These tests are still valuable as they revealed a lot of issues initially.
New errors however are rare and these tests should just guard against issues creeping in over time.
Testing once a week on master should be sufficient to keep the codebase healthy.
The caching mechanism in place to speed up retries will also help here.
If there were no changes in the past week this job will skip each step.
2 : Partial tests.
test-modified-only
is used as an argument fornode ./test/polyfills/remotetest.js
.master
onhttps://github.com/Financial-Times/polyfill-library.git
It doesn't take
origin/master
to make it easier for contributors who fork and open pr's. Their CI should not test against their ownmaster
but againstupstream/master
Notes :
This is a rather big change and I would like to revisit it in the next couple of days with fresh eyes.
Any feedback in the mean time is more than welcome!
If there is a reason not to implement a feature like this at all, that is totally fine too! 🙂
This PR would be a good validation of this change as it's only needed in a few old browser : #991