Skip to content

Conversation

@kripken
Copy link
Member

@kripken kripken commented Nov 30, 2017

Split out the part of other.test_emcc that checks for a version mismatch, which currently fails on CI since it uses 1.37.22 instead of 1.37.23. Disable that test on CI for now.

I also merged the skip lists, which were in two places for CircleCI - was there a reason for that I am missing?

…tch, which currently fails on CI since it uses 1.37.22 instead of 1.37.23. disable that test on CI for now
@kripken kripken requested a review from saschanaz November 30, 2017 18:41
@saschanaz
Copy link
Collaborator

saschanaz commented Dec 1, 2017

I think test_emcc_v is a typo, right? Oops, it was the right name.

Both places work, but I prefer adding those to each TEST_TARGET as otherwise the skip commands apply for every test job. Adding to TEST_TARGET makes the command apply only for the target jobs so that we can see shorter logs.

I kept those SSE skipping commands merged as I thought it would be easier to remove them later. It might be better to split them for consistency.

@saschanaz
Copy link
Collaborator

saschanaz commented Dec 1, 2017

No idea why test_emcc_multiprocess_cache_access fails. (It's not a cache lock problem, a process just failed but the test doesn't print stderr.)

@kripken
Copy link
Member Author

kripken commented Dec 1, 2017

On the other hand, adding the skips to them all leaves all the skips on one line, or adjacent lines (as in this PR), which seems nice? Or did I misunderstand you?

@saschanaz
Copy link
Collaborator

On the other hand, adding the skips to them all leaves all the skips on one line, or adjacent lines (as in this PR), which seems nice?

Um, what do you mean by 'adding the skips to them all'?

@saschanaz
Copy link
Collaborator

saschanaz commented Dec 1, 2017

So test_emcc_multiprocess_cache_access just randomly failed. 🤔

(Failure log: https://circleci.com/gh/kripken/emscripten/115)

@kripken
Copy link
Member Author

kripken commented Dec 1, 2017

Yeah, sorry, I wasn't clear. I meant the state in this PR, where the skips are all on line 20 of the circleci yml file. But yeah, it does mean they get added to all the logs. I don't feel strongly about this, either way is ok.

I saw that random failure too. Weird. That should use file locks to ensure libc gets built once. I don't know exactly how those locks work, but I've never seen them fail. Maybe we have a bug in the lock code? (tools/filelock.py)

@saschanaz
Copy link
Collaborator

I prefer shorter-log way but either way is okay for now, as anyway those three skips will soon be removed when the new SDK is built. Probably within a week, or when @juj returns.

test_emcc_multiprocess_cache_access (test_other.other) ... Process Process-1:
Traceback (most recent call last):
  File "/usr/lib/python2.7/multiprocessing/process.py", line 258, in _bootstrap
    self.run()
  File "/usr/lib/python2.7/multiprocessing/process.py", line 114, in run
    self._target(*self._args, **self._kwargs)
  File "/root/emscripten/tests/test_other.py", line 13, in multiprocess_task
    output = subprocess.check_output([PYTHON, EMCC, c_file, '--cache', cache_dir_name], stderr=subprocess.STDOUT)
  File "/usr/lib/python2.7/subprocess.py", line 574, in check_output
    raise CalledProcessError(retcode, cmd, output=output)
CalledProcessError: Command '['/usr/bin/python', '/root/emscripten/emcc', '/tmp/emscripten_test_emcache_wmSP3P/test.c', '--cache', '/tmp/emscripten_test_emcache_wmSP3P/emscripten_cache']' returned non-zero exit status 1

Couldn't repro this error on my local machine, we should keep an eye for this...

@kripken
Copy link
Member Author

kripken commented Dec 1, 2017

I pushed a version with separate skips as you suggested, after reading more of the logs I agree it's nicer when they are shorter :)

@saschanaz
Copy link
Collaborator

Those SSE-related tests may not be fixed even after the new SDK build as they fail because the SDK-provided native clang cannot find SSE headers.

@saschanaz saschanaz mentioned this pull request Jan 30, 2018
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.

3 participants