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

test for unknown actions in TDS #60

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ladamski
Copy link
Collaborator

https://app.asana.com/0/1171671347221384/1203125805176391/f

Tests for unrecognized custom "action"s in tracker radar entries. This cannot be merged until we fix JSON test parsing in MacOS browser (at minimum), otherwise it will fail at file load. Can be tested with the extension once PR is merged

Copy link
Member

@kdzwinel kdzwinel left a comment

Choose a reason for hiding this comment

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

Thanks for writing those! Not sure why we should not merge this? Will BSK pull those tests automatically and block releases until this is fixed? Do you know if we have the same issue on Android and Windows?

@ladamski
Copy link
Collaborator Author

ladamski commented Oct 20, 2022

@kdzwinel we can definitely merge it, its just that it would essentially freeze those products at the current ref test HEAD, and so they would miss out on other test updates until they are updated to handle this case.

@jonathanKingston
Copy link
Collaborator

its just that it would essentially freeze those products at the current ref test HEAD, and so they would miss out on other test updates until they are updated to handle this case.

We're heading to auto updating; we should mark them as failures or similar on the other platforms.

@ladamski
Copy link
Collaborator Author

ladamski commented Nov 1, 2022

its just that it would essentially freeze those products at the current ref test HEAD, and so they would miss out on other test updates until they are updated to handle this case.

We're heading to auto updating; we should mark them as failures or similar on the other platforms.

I'm happy to merge this if that's the consensus, it will however break TDS file parsing so tests will just fail hard, no way to create a platform exception for that without fixing the issue.

@jonathanKingston
Copy link
Collaborator

it will however break TDS file parsing so tests will just fail hard, no way to create a platform exception for that without fixing the issue.

So the individual cases can't be disabled just the entire suite?

If that's the case: An alternative is making a new suite purely for v4 and disabling on all other platforms for now.

  • This would give us coverage of v3 on all other platforms and allow us to continue to land other tests also whilst also getting this tested on extension v4.

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