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

fix same file access exception #3373

Merged
merged 7 commits into from
May 30, 2022
Merged

fix same file access exception #3373

merged 7 commits into from
May 30, 2022

Conversation

ntovas
Copy link
Contributor

@ntovas ntovas commented Feb 15, 2022

The current pull request fixes the following error: Html Logger Error : The process cannot access the file 'C:\xxxxxxx\TestResult_XXXXXXXXXX_20220215_084850.xml' because it is being used by another process., while running tests for multiple projects on the same solution.

@nohwnd
Copy link
Member

nohwnd commented Feb 16, 2022

Thanks for your contribution :)

@Haplois imho we had a code for this somewhere, could you remind me? Or were you doing that fix in testfx?

@ntovas
Copy link
Contributor Author

ntovas commented Feb 16, 2022

@nohwnd in TrxFileHelper there is code that has the same functionality, I wanted to make something common and reusable, but I don't have any familiarity with the codebase.

@nohwnd
Copy link
Member

nohwnd commented Feb 16, 2022

@ntovas perfect. 👍

@defilerc
Copy link

@nohwnd Hi! Is there any chance you plan to merge this fix soon? The fix follows the same logic used in TrxFileHelper mentioned above by @ntovas .

@nohwnd
Copy link
Member

nohwnd commented Apr 14, 2022

Thanks for reminding me about this. Added to my work list for Tuesday.

@ntovas
Copy link
Contributor Author

ntovas commented Apr 14, 2022

I reviewed my code and made some changes, please check them again @nohwnd :)

@ntovas ntovas force-pushed the same-file-fix branch 2 times, most recently from 9bfa600 to 79da441 Compare April 14, 2022 20:14
Copy link
Member

@Evangelink Evangelink left a comment

Choose a reason for hiding this comment

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

Looks like some of the acceptance tests are failing (probably because of the name change), tests are:

  • HtmlLoggerWithExecutorUriShouldProperlyOverwriteFile
  • HtmlLoggerWithFriendlyNameShouldProperlyOverwriteFile

@Evangelink
Copy link
Member

Hey @ntovas, would you be willing to make the extra changes so we can move forward and get this PR merged? If not, please let us know so we can take it over from here.

Thank you for what you have done so far!

@ntovas
Copy link
Contributor Author

ntovas commented May 23, 2022

Hello @Evangelink ,

Yes, I will try to make the changes, sorry but I missed the notifications about this.

@Evangelink
Copy link
Member

No worries at all @ntovas. Thank you for taking care of this and feel free to ask any question.

@ntovas ntovas force-pushed the same-file-fix branch 2 times, most recently from 8c38c73 to 7f7e996 Compare May 23, 2022 19:24
@ntovas
Copy link
Contributor Author

ntovas commented May 23, 2022

@Evangelink I have an issue running the unit tests locally, so I am not exactly sure why it's failing ¯_(ツ)_/¯

@Evangelink
Copy link
Member

@ntovas I will look into it. Thanks for updating the PR :)

@Evangelink Evangelink self-assigned this May 24, 2022
ex.ToString());
ConsoleOutput.Instance.Error(false, string.Concat(HtmlResource.HtmlLoggerError), ex.Message);
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated change but there was no need for the string.Concat.

@nohwnd nohwnd merged commit 3330a06 into microsoft:main May 30, 2022
@Evangelink
Copy link
Member

Thank you for the contribution @ntovas!

@ntovas ntovas deleted the same-file-fix branch May 31, 2022 09:21
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.

4 participants