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

Adding missing assemblies. #39

Merged
merged 4 commits into from
Sep 18, 2024
Merged

Adding missing assemblies. #39

merged 4 commits into from
Sep 18, 2024

Conversation

UL-ChrisGlew
Copy link
Contributor

🤔 What's changed?

Adding missing dependency files:

  • TestableIO.System.IO.Abstractions.dll
  • TestableIO.System.IO.Abstractions.Wrappers.dll

That are now required due to an update in the System.IO.Abstractions package. I have tested this on an envrionment where #37 could be replicated. Tested both the broken VSIX file and one created with the lines ammended, which seems to function correctly.

⚡️ What's your motivation?

To fix #37

🏷️ What kind of change is this?

  • 🐛 Bug fix (non-breaking change which fixes a defect)

♻️ Anything particular you want feedback on?

N/A

📋 Checklist:

  • [*] Users should know about my change
    • [*] I have added an entry to the "[vNext]" section of the CHANGELOG, linking to this pull request & included my GitHub handle to the release contributors list.

This text was originally taken from the template of the Cucumber project, then edited by hand. You can modify the template here.

@UL-ChrisGlew UL-ChrisGlew reopened this Sep 18, 2024
Copy link
Contributor

@gasparnagy gasparnagy left a comment

Choose a reason for hiding this comment

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

Fantastic. Thx for the quick analysis and fix. You are the hero of the day! 🤘

@gasparnagy gasparnagy merged commit 22e7e29 into main Sep 18, 2024
2 checks passed
@gasparnagy gasparnagy deleted the Maintenance/Fixing-37 branch September 18, 2024 15:26
@gasparnagy
Copy link
Contributor

@UL-ChrisGlew When you played with it locally, have you observed any issues with the Diagnostics.LoggingTests class from the unit test suite? We have a flaky error there, see #40.

@UL-ChrisGlew
Copy link
Contributor Author

I hadn't initially - have re-run the unit tests a few times and had one test fail in that time due to #40 - so does seem to be a flaky error.

@UL-ChrisGlew
Copy link
Contributor Author

I've just run them again and set to run until failure, and have not had any logging tests fail in 78 iterations - the failure that I have got was not related to logging:
image

@gasparnagy
Copy link
Contributor

Hm... that is something else. From the "Specs" project there might be such errors sometimes - not good, but that are integration tests. But the worrying about the logger tests that they are unit tests, and (at least on the CI) more often happen. Just to double check: When you used "run until failure" have you done that for the Reqnroll.VisualStudio.Tests project?

One more: Could you please try to upgrade the ApprovalTests dependency to 6.0.0 and see if the flakiness comes for the logger tests?

@UL-ChrisGlew
Copy link
Contributor Author

Yes, I've been running the whole Reqnroll.VisualStudio.Tests as 'Run until failure' and am not seeing any failures around those logging tests at the moment. I left just those logging tests running for ~500 iterations and they've not yet failed.

Have just updated the ApprovalTests package and ran again, failed after 11 iterations but I don't think it's related?

image

@gasparnagy
Copy link
Contributor

I think it's just coincidence, but who knows...

@gasparnagy
Copy link
Contributor

This particular error I have seen already a few times already in SpecFlow times...

@UL-ChrisGlew
Copy link
Contributor Author

Ran again and have a different failure:

image

Again, don't think this is related?

@gasparnagy
Copy link
Contributor

Hm... Maybe these are also more frequent now. Anyhow. It seems we have quite some flakiness in our tests, so we will need to dig into these one after the other...

Maybe what happened is that the unit test framework has been improved to run the test parallel slightly differently, and now more of these come up.

@gasparnagy
Copy link
Contributor

The question is whether we should wait with releasing the fixed dependencies. Because these seem to be more of testing issues and not related to the product itself.

@UL-ChrisGlew
Copy link
Contributor Author

I don't think it's related to the fixed dependencies - have just checked out the project prior to the package updates going through and am still getting some sporadic failures in the unit tests:

image

@gasparnagy
Copy link
Contributor

OK. Could you please copy this last comment to #40 and let's track the flaky problems there.

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.

Reqnroll Extension v2024.3.152 does not work on Visual Studio 17.11.4
2 participants