Skip to content

bug fix in build.py which prevents running some unit tests#5990

Merged
thiagocrepaldi merged 2 commits intomasterfrom
bmeswani/build_bug
Dec 3, 2020
Merged

bug fix in build.py which prevents running some unit tests#5990
thiagocrepaldi merged 2 commits intomasterfrom
bmeswani/build_bug

Conversation

@baijumeswani
Copy link
Contributor

Description:
Argument to build.py --minimal_build is a string argument. But it is being used as a bool where there is a bug. So, some of the tests are not being run as they should be in the ci pipelines.

@baijumeswani baijumeswani requested a review from a team as a code owner December 1, 2020 22:23
@snnn snnn requested a review from skottmckay December 1, 2020 22:35
snnn
snnn previously approved these changes Dec 1, 2020
thiagocrepaldi
thiagocrepaldi previously approved these changes Dec 2, 2020
@baijumeswani
Copy link
Contributor Author

Some more background:
It seems like this bug was introduced in #5776 here where the bool argument --minimal_build was changed to a string argument.
But while doing so, one place where this argument was used (introduced in #5056 here) was missed.
This led to the python tests from not being executed in the pipeline.

So far, tests would have passed but were not being executed in the pipeline.

Now, with changes from #5840, this line, would have caused one of the unit tests to fail (TestInferenceSession.testModelSerialization). But because these tests were not running in the pipeline, this failure was not caught.

From what I gather, this model serialization will throw an error for any execution provider that generate compiled nodes. And because of this, this test fails. So, I made the change to catch the exception thrown and compare it against the message "[ONNXRuntimeError] : 1 : FAIL : Unable to serialize model as it contains compiled nodes. Please disable any execution providers which generate compiled nodes." and pass the test if these two strings are equal. In short, this test should not be run when the exception thrown matches "[ONNXRuntimeError] : 1 : FAIL : Unable to serialize model as it contains compiled nodes. Please disable any execution providers which generate compiled nodes.".

@baijumeswani baijumeswani requested a review from askhade December 3, 2020 04:41
@thiagocrepaldi thiagocrepaldi merged commit 2b35f7d into master Dec 3, 2020
@thiagocrepaldi thiagocrepaldi deleted the bmeswani/build_bug branch December 3, 2020 16:57
@baijumeswani
Copy link
Contributor Author

Thank you for the review. 😃

snnn pushed a commit that referenced this pull request Dec 4, 2020
Also ignore an exception occurred for execution providers which generate compiled nodes
snnn pushed a commit that referenced this pull request Dec 9, 2020
Also ignore an exception occurred for execution providers which generate compiled nodes
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