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

Keep overridden sysdef in same directory #36

Merged
merged 2 commits into from
Apr 9, 2019
Merged

Conversation

buckd
Copy link
Collaborator

@buckd buckd commented Apr 9, 2019

What does this Pull Request accomplish?

Generates overridden system definition in the same location as the original system definition so any relative paths in the definition can still be located.

Why should this Pull Request be merged?

Tests requiring dependent files will fail to deploy without changes to ensure relative paths in the sysdef are consistent.

What testing has been done?

image

@buckd buckd requested review from csjall and rtzoeller as code owners April 9, 2019 18:05
@niveristand-diff-bot
Copy link
Collaborator

Bleep bloop!

LabVIEW Diff Robot here with some diffs served up hot for your pull request.

Notice something funny? Help fix me on my GitHub repo.

VeriStandTestUtilities.lvlib--GenerateOverriddenSystemDefinition.vi.png

capture

The following VIs could not be diffed:

  • C:\jenkins\workspace-device-testing-tools_PR-36-6VZK764BEV4AGDLMHFZV3VF6ZNJICOCEEL2ZHN2QFYSYUHZF6NXQ\VeriStandTestCase\CloseVeriStandConnection.vi
  • C:\jenkins\workspace-device-testing-tools_PR-36-6VZK764BEV4AGDLMHFZV3VF6ZNJICOCEEL2ZHN2QFYSYUHZF6NXQ\VeriStandTestCase\Utilities\PatchSystemDefinitionFile.vi

@rtzoeller
Copy link
Contributor

It looks like there is a potential race condition in CloseVeriStandConnection.vi. We are deleting a file that VeriStand is using, without explicitly disconnecting/undeploying first.
image

In practice I think this is ok, because I don't expect VeriStand to be constantly re-reading the file from disk, but IMO we should not assume this.

@buckd buckd merged commit 4389157 into master Apr 9, 2019
@buckd buckd deleted the dev/temp-file-fix branch April 9, 2019 18:37
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