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

Change Proposal: Make a Polymorphic VI for Assert (Request for Comments) #26

Closed
jimkring opened this Issue Oct 18, 2018 · 7 comments

Comments

Projects
3 participants
@jimkring
Copy link
Contributor

jimkring commented Oct 18, 2018

Background / Problem Statement
There are a lot of different Assert VIs. The only way, currently, to know which ones are available is by looking on the palette.

image

image

Often, one wishes to change an existing call to a different kind of Assert -- e.g. Assert Equal to Assert Not Equal, or Assert Equal Value to Assert Equal Value and Type.

image

Proposed Solution / Poly VI
A Polymorphic VI that shows it's selector right could make it easier for users to both (A) discover the different types of assertions and (B) replace an existing call with a different type of assertion, using the poly selector ring. Additionally, if the poly selector ring is visible by default, it will clearly show which instance is being called, so that the user does not have to rely purely on the icon of the assertion instance to know which assertion is being called.

The following screenshot shows an example of what would be possible (however, the actual names and poly menu structure shown are just examples and not fully thought out)
image

Also, we could remove all the instances of Assert from the palette and simply have a single Assert VI.

image

@jimkring jimkring self-assigned this Oct 18, 2018

@jimkring jimkring changed the title Change Proposal: Make a Polymorphic VI for Assert (Please Discuss) Change Proposal: Make a Polymorphic VI for Assert (Request for Comments) Oct 18, 2018

@francois-normandin

This comment has been minimized.

Copy link
Collaborator

francois-normandin commented Oct 18, 2018

I really like this proposal.

  • It's clean and educational for new users of the framework.
  • It's easier to use with QuickDrop (we should try to make this poly VI appear at top of assertions list when quick-dropping)
  • I'd vote for the "Assert True" to be the default drop-in type
@kosist

This comment has been minimized.

Copy link

kosist commented Oct 18, 2018

Cool idea, it would simplify work with the tool a lot.

@francois-normandin francois-normandin moved this from In progress to In Review in Next Release (0.6.0) Oct 24, 2018

@francois-normandin francois-normandin moved this from In Review to Ready for Release in Next Release (0.6.0) Oct 24, 2018

@kosist

This comment has been minimized.

Copy link

kosist commented Oct 24, 2018

I have changed my mind, I didn't think it though before well...
Don't know, whether it'll be interesting for you, but let me just give some small feedback after testing with this release candidate 0.6.0.

One the one hand, it is nice to have single, poly VI which allows to select any available assert.
But on the other hand, it requires more "clicks" to get what one needs.
Just check it (without quick-dropping):

  1. Go to JKI Toolkits menu.
  2. Select "Caraya Test Framework".
  3. Select Assert (Poly).vi, drop on block diagram.
  4. Click on Polymorphic VI selector.
  5. Navigate to 3 levels: Compare -> Equal -> Value.

Now we're done.

Although, with no-polymorphic VI, it could be done within 3 upper mentioned steps, without additional mouse clicks.
Moreover, when VIs are separated, it is possible to nicely configure them for quick drop; and select by it
most often used asserts.
With polymorphic VI, it is possible to call just polymorphic VI -> and then anyway one needs to configure it manually.

Maybe, there could be done compromise, and there could be left both approaches on the function pallete - poly VI; and separated VIs?

@francois-normandin

This comment has been minimized.

Copy link
Collaborator

francois-normandin commented Oct 25, 2018

Here's what the proposed compromise looks like.

Please comment.

image

Should we change the icon color for the Poly Assert, to make it stand out a bit?

@kosist

This comment has been minimized.

Copy link

kosist commented Oct 25, 2018

Thanks a lot! Looks nice.
Seems, that same color will be fine - because anyway on the functions pallete Poly vi stands a bit separately (in the upper row), so it already stands out from other asserts.
Moreover, if someone will use on the same block diagram "classic" assert VIs and polymorphic VIs - then it will be in the same visual style.

P.S. Maybe, it worth to share information about these changes on NI forum (Unit Testing Group), to receive more feedbacks? Because I guess not all the users of Caraya framework check github...

@francois-normandin

This comment has been minimized.

Copy link
Collaborator

francois-normandin commented Oct 25, 2018

FWIW, the Polymorphic VI's icon is just shown in the palette... once dropped on the block diagram, it takes the icon of whatever flavor you choose, so a different color would just highlight it.

Sharing on the different forums is a great idea. I propose that we let JKI and @jimkring review the latest code, and then we can publicize the next build as "Beta Release" to gather comments from the larger community before pushing it into "production".

@francois-normandin

This comment has been minimized.

Copy link
Collaborator

francois-normandin commented Dec 7, 2018

Implemented in the stable release of 0.6.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment