Conditional rendering testing value of object of forward/backward link

[jg10-mastodon-social]

Closes #184

• Add some-value-eq and every-value-eq
• Add Thing.observeLiterals
• Support literals for if-property
• Add (some|every)-value-(gt|gte|lt|lte)
• Tests have been written
  • all new code is covered by unit tests
  • the happy path of a new feature is covered by an end-to-end test
    • added to pos-switch integration test
  • manual explorative tests have been performed
• all dependencies are updated to the latest patch version at minimum
• the CI pipeline passes
• documentation is up-to-date
  • TSDoc style comments on important functions, properties and events
  • stories for new PodOS elements have been added to storybook
  • Readme.md files of PodOS elements have been re-generated
  • N/A- architectural decisions are documented as an ADR
  • Changelogs are updated according to Keep a Changelog

[jg10-mastodon-social]

I suspect we might need more tests here, but I'm not sure how to design them.
I designed these as TDD tests - they all fail if the numbers are treated as strings.

[jg10-mastodon-social]

I've now grouped tests, and split out tests specifically of the test method.
I've tried to systematically go through possible combinations of inputs and data, using the it.each approach.

The code is still missing systematic tests of:

• evaluating values of property and rev with multiple relations
• evaluating values of property with multiple strings
• evaluating values of property with multiple numbers

There are ad-hoc tests for this functionality, so given the difficulty in parameterising these combinatorial tests, I suggest we open an issue documenting this gap and merge in the mean time.

Any suggestions are welcome for systematically testing all combinations in the presence of multiple values!

[jg10-mastodon-social]

I think this is now 0.30.0, but I may have misunderstood what is in the previous release

[angelo-v]

you are right, next version will be 31. I might have messed up the changelog on main, will check that

[jg10-mastodon-social]

@angelo-v Is this correct?
When rebasing on main, it looked like your previous fixes no longer applied given the release in the mean time?

[angelo-v]

this looks correct, version in package json is "0.42.0-next.0" so 0.42.0 will be the next released version. If you rebase onto main and 0.42.0 was meanwhile released you will see the version increase to 0.43.0-next.0 and you then need to adjust the changelog accordingly

[angelo-v]

@jg10-mastodon-social Sorry, I just noticed I left some comments in "pending" state and you propbably did not see them. I am just submitting them now - yet I did not find the time to look at your latest changes, so I am not sure if they are still relevant. I am going to continue reviewing the current state later this week

[angelo-v]

issue: this function is very complex and should a) be refactored into smaller chunks and b) moved into its own file and heavily unit tested. Comparision logic be separated from element attributes.

[jg10-mastodon-social]

I hadn't seen this comment and have therefore not refactored. Any help in dividing it up would be great!

I did however create a separate test file for this function so (b) is partially addressed.

I'm not sure about separating out comparison logic from element attributes - the main purpose of the function is to apply all necessary combinations of element attributes - the comparison logic itself is arguably trivial once the effect of the element attributes is applied.

[angelo-v]

my fault! I did not submit it properly and am very sorry. I added a refactoring commit that goes into the direction I tried to explain, but it is still a long way to go. I am sorry to say that but in the current state the complexity in pos-switch is unacceptable to me and propably still a source of many bugs.

Thank you very much for the extensive test suite you added. These tests are a necessary security net for refactoring. Yet the combinatory explosion of these tests and the fact that they are yet not able to cover every case are a big big smell.

pos-switch does effectifly three things:

1. data gathering: find all the necessary data for comparision (observed relations, values etc)
2. rules: gather semantics, operators and their respective values applied to pos-case (like "some-value-eg=..." etc.)
3. apply the rules to the gathered data

I imagine that those 3 parts could be well separated and tested by themselves and this would reduce the testing combinatoric a lot

[angelo-v]

I am thinking of something like that:

const data = {
  literals: [],
  relations: [],
  reverseRelations: [],
  types: []
};

const rule = {
  type: "if-property",
  value: "https://schema.org/name",
  operator: "eq",
  semantic: "some",
  target: "Alice"
}

function ruleMatches(r: typeof rule, d: typeof data): boolean {
}

1. collect data from resource
2. create rule(s) from dom attributes
3. match rules with data

ruleMatches is a pure function with no dependency on dom or resource that could be easily testable. Also there can be multiple sub-functions for different rule types that can be tested and focus on that specific rule

[angelo-v]

There is Element.getAttributeNames that might make it easier to get the attributes from dom compared to checking if each exists individually, but unfortunately stencil testbed does not implement it. This should not stop us from using it, if it helps us. If the pure function based on configured rules does the heavy logic we can easily mock getAttributeNames for the component testing. I am also currently migrating to vitest and this might make the stencil testing situation better.

[angelo-v]

vitest is available on main branch now, so you can add a vitest spec by using *.vspec.tsx if that helps

[angelo-v]

issue: same as above. Try to collect the relevant data and condition from component first, then do the logic in a dedicated function that is independent from the component and well tested