Skip to content
This repository was archived by the owner on Jul 15, 2023. It is now read-only.

Conversation

@tom-englert
Copy link
Contributor

Fix the Analyze1FromSourcesV40 regression tests. (for #148)

  • offset changes
  • changes due to fixed system contracts

Should not be pulled before #152, #153 and #154

@SergeyTeplyakov
Copy link
Contributor

I'm going to merge this stuff. Any objections? @tom-englert, @sharwell

@sharwell
Copy link
Contributor

sharwell commented Aug 3, 2015

Merging #152, #153, and #154 should not be a problem. After that, we will need to rebase this pull request before it can be merged.

@tom-englert
Copy link
Contributor Author

I will rebase this as soon as the others are merged.

@SergeyTeplyakov
Copy link
Contributor

I assume that the fix was rebased. If there is no objections I'll merge the PR.

@sharwell
Copy link
Contributor

@SergeyTeplyakov No, this pull request is still waiting for #152, #153, and #154 to be merged.

@SergeyTeplyakov
Copy link
Contributor

Merged #152, #153, and #154.

@sharwell
Copy link
Contributor

👍 We'll take a look at updating this now 😄

@tom-englert
Copy link
Contributor Author

Rebased, ready to pull 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

❗ Please revert this header change (and any like it).

Edit: This is part of the first three formatting commits which can simply be dropped from this pull request. 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So the two lines header is the correct one?

@sharwell
Copy link
Contributor

❗ Can you rebase this to not include the first three commits?

Signed-off-by: tom-englert <mail@tom-englert.de>
…ad to InvariantCulture, so everything behaves the same as in the US.

Signed-off-by: tom-englert <mail@tom-englert.de>
@tom-englert
Copy link
Contributor Author

OK, just committed the functional changes.
I thought the formatting changes would be gone after merging automatically, but somehow they survived.

@sharwell
Copy link
Contributor

Changes to fix the tests look fine. I would prefer to separate the culture change to its own pull request, but I'll leave the call up to @SergeyTeplyakov. Overall 👍.

@tom-englert
Copy link
Contributor Author

p.s. there is already an issue to make string-formatting culture-independent: #149

Copy link
Contributor

Choose a reason for hiding this comment

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

This file causes ClousotTests not to show in Test Explorer, because it contains methods declared extern.

Error loading C:\…\CodeContracts\Microsoft.Research\RegressionTest\ClousotTests\bin\Debug\ClousotTests.dll: Unable to load the test container 'C:\…\CodeContracts\Microsoft.Research\RegressionTest\ClousotTests\bin\Debug\ClousotTests.dll' or one of its dependencies. Error details:
System.TypeLoadException: Could not load type 'TestPostconditionInference.FilterRedundantPostcondition' from assembly 'ClousotTests, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null' because the method 'GetApplicationVersion' has no implementation (no RVA).
System.TypeLoadException: Could not load type 'TestPostconditionInference.Array' from assembly 'ClousotTests, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null' because the method 'get_Length' has no implementation (no RVA).

Copy link
Contributor

Choose a reason for hiding this comment

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

@sharwell I prefer separate PR, but separate commits are fine for now.

@tom-englert What do you think about @panacekcz comment? We can fix this in separate commit/PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These files should have been added as <None>, not as <Compile>.
It's only to have the test files part of the project.

SergeyTeplyakov added a commit that referenced this pull request Aug 15, 2015
Fix regression tests (Analyze1FromSourcesV40)
@SergeyTeplyakov SergeyTeplyakov merged commit 1f13c58 into microsoft:master Aug 15, 2015
@tom-englert tom-englert deleted the issue148 branch August 28, 2015 18:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants