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

Resolved test failure of test_DSF_Reentrancy #93

Merged
merged 3 commits into from
Apr 24, 2024

Conversation

Pritamp99
Copy link
Contributor

@Pritamp99 Pritamp99 commented Apr 19, 2024

What does this Pull Request accomplish?

Changes: Skip "test_DSF_reentrancy"

Saved in LabVIEW 2021 64-bit.

Why should this Pull Request be merged?

It resolves the issue ERROR 1390 which was thrown because of out of Scope calling of a Top.vi.

Skipping the test for now.

What testing has been done?

  • None

@Pritamp99 Pritamp99 self-assigned this Apr 19, 2024
@Pritamp99 Pritamp99 requested a review from buckd as a code owner April 19, 2024 15:32
@buckd
Copy link
Collaborator

buckd commented Apr 22, 2024

When I run the test locally, it still fails with the same error in LabVIEW 2021. Can you test it in 2021 and see if this fix works?

@buckd
Copy link
Collaborator

buckd commented Apr 23, 2024

I went back and looked at when that test was added to the ATS (23.3 build 8). That test has never passed. It was added 4 years ago and, presumably, worked when it was added (#22) . But, there have been numerous changes to both VI Tester and LabVIEW, particularly related to classes and PPLs since then, so I'm not completely surprised by this. It was added as a check for the framework to be utilized with multiple instances in case someone wanted to use it outside the custom device.

In VeriStand, you can only add one instance of the custom device per target, so the case this test is checking is not a valid use case for VeriStand and I'm not aware of anyone using the framework outside the custom device. Given this information, I propose we either skip or delete this test.

@Pritamp99
Copy link
Contributor Author

Skipped the "test_DSF_Reentrancy" for now.

@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.

Reentrancy Tests.lvclass--test DSF Reentrancy.vi.png

capture

@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.

Reentrancy Tests.lvclass--test DSF Reentrancy.vi.png

capture

Copy link
Collaborator

@buckd buckd left a comment

Choose a reason for hiding this comment

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

Please update the description of the PR because it still says there were friend relationships added between the thread classes and that's not true anymore.

@Pritamp99 Pritamp99 merged commit d0d1836 into main Apr 24, 2024
7 checks passed
@Pritamp99 Pritamp99 deleted the users/ppal/test_DSF_Reentrancy_issue branch April 24, 2024 05:42
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