Skip to content

Move test_exceptions_white_list_uncaught into tests/core.#10776

Merged
sbc100 merged 1 commit intomasterfrom
move_exception_test
Mar 27, 2020
Merged

Move test_exceptions_white_list_uncaught into tests/core.#10776
sbc100 merged 1 commit intomasterfrom
move_exception_test

Conversation

@sbc100
Copy link
Copy Markdown
Collaborator

@sbc100 sbc100 commented Mar 26, 2020

I'm not sure why this was originally added under tests/third_party but
it looks like a mistake.

I'm not sure why this was originally added under tests/third_party but
it looks like a mistake.
@sbc100 sbc100 requested a review from aheejin March 26, 2020 16:29
@aheejin
Copy link
Copy Markdown
Member

aheejin commented Mar 26, 2020

It's because of @kripken's suggestion here: #10720 (comment). But I also think that the program is so simple and I even modified it a bit to make it work with Emscripten, so it can be moved to tests/core. WDYT @kripken?

@sbc100
Copy link
Copy Markdown
Collaborator Author

sbc100 commented Mar 26, 2020

Ah.. I see...interesting. I'm not sure it makes sense to have individual tests and expectations under third_party.

@kripken
Copy link
Copy Markdown
Member

kripken commented Mar 26, 2020

I'm fine either way. But I thought @sbc100 you were advocating for tests/third_party in general, which is why I put coremark under there? Are you saying that's just for entire projects but not individual files?

@sbc100
Copy link
Copy Markdown
Collaborator Author

sbc100 commented Mar 26, 2020

I do want third party code in general to be in third_party but I would expect any of our test entry points or the .out files to live there. I'm not sure this is a significant enough a piece of code to warrant this separation.

@sbc100 sbc100 merged commit fb64aea into master Mar 27, 2020
@sbc100 sbc100 deleted the move_exception_test branch March 27, 2020 00:52
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