Skip to content
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

Add the ability to save report output to a file #509

Merged
merged 1 commit into from Mar 11, 2015
Merged

Add the ability to save report output to a file #509

merged 1 commit into from Mar 11, 2015

Conversation

stabbylambda
Copy link
Contributor

This adds the ability to save the report output to a file in CI mode. I added this because I'd like the xml output for my Jenkins jobs. Normally I would just pipe the output from testem ci to a file, but I'm running testem through ember-cli, which prints some text to stdout before running testem.

Let me know if anything looks out of order!

@stabbylambda
Copy link
Contributor Author

Alright. This seems to fail randomly on Windows. It's working on my VM, but for some reason AppVeyor is failing at different points in my test suite and I'm not sure what's up.

@johanneswuerbach
Copy link
Member

Thank you @stabbylambda, could you squash and rebase your commits? master is using now Node 0.12 in Windows, which actually prints the test error message.

filename = getFileName()
})
afterEach(function(done) {
rimraf(filename, done)
Copy link
Member

Choose a reason for hiding this comment

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

Sorry for my late response, but GitHub doesn't send a notification when only commits in a PR changed.
Could you check, whether the file exists? rimraf should do it, but doesn't seem to work.

Otherwise, you might need to use tempfiles for testing https://www.npmjs.com/package/tmp

@stabbylambda
Copy link
Contributor Author

🙏 Alright. I think this is good to go. I've decided that I still hate Windows. 🔫

@johanneswuerbach
Copy link
Member

Windows CI run into an npm issue, restarted.... Windows 💥

})
})
after(function(done) {
rimraf(mainReportDir, function() {
Copy link
Member

Choose a reason for hiding this comment

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

This could be removed, when tmp.dir({unsafeCleanup: true}, cb) is used in before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried that and it was still throwing crazy errors on Windows and folders were still hanging around.

On Mar 4, 2015, at 12:10 AM, Johannes W?rbach <notifications@github.commailto:notifications@github.com> wrote:

In tests/ci/report_file_tests.jshttps://github.com//pull/509#discussion_r25757807:

+var path = require('path')
+var PassThrough = require('stream').PassThrough
+var ReportFile = require('../../lib/ci/report_file')
+var XUnitReporter = require('../../lib/ci/test_reporters/xunit_reporter')
+var tmp = require('tmp')
+
+describe('report file output', function() {

  • var mainReportDir, reportDir, filename;
  • before(function(done) {
  • tmp.dir(function(err, path) {
  •  mainReportDir = path
    
  •  done()
    
  • })
  • })
  • after(function(done) {
  • rimraf(mainReportDir, function() {

This could be removed, when tmp.dir({unsafeCleanup: true}, cb) is used in before.

Reply to this email directly or view it on GitHubhttps://github.com//pull/509/files#r25757807.

@BryanCrotaz
Copy link
Contributor

Reviewed the code, looks good. I have the same requirement for ember-cli projects

@johanneswuerbach
Copy link
Member

I would love to merge this asap, but we need to make the windows tests passing. I won't have internet until Friday, so can't look into this before the weekend.

johanneswuerbach added a commit that referenced this pull request Mar 11, 2015
Add the ability to save report output to a file
@johanneswuerbach johanneswuerbach merged commit 2dcc0b7 into testem:master Mar 11, 2015
@johanneswuerbach
Copy link
Member

Restarted windows ci and it passed 💥 Thank you 👍

@stabbylambda
Copy link
Contributor Author

Sweet! Restarting Windows fixes everything!

@cgartner
Copy link

I'm sorry for dragging this up but I can't find the solution documented anywhere. Just what needs to be added to the testem.json or command line arguments to make this work?

@johanneswuerbach
Copy link
Member

@cgartner you can set the report_file setting in your testem.json

@cgartner
Copy link

Thank you @johanneswuerbach that did the trick!

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.

None yet

4 participants