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

Update list tests to not need to have the full output in the test #437

Closed
2 tasks done
andrewtavis opened this issue Oct 19, 2024 · 18 comments
Closed
2 tasks done

Update list tests to not need to have the full output in the test #437

andrewtavis opened this issue Oct 19, 2024 · 18 comments
Assignees
Labels
feature New feature or request good first issue Good for newcomers hacktoberfest Included as a part of Hacktoberfest help wanted Extra attention is needed

Comments

@andrewtavis
Copy link
Member

Terms

Description

An issue with the current tests of list for the CLI is that they require us to update their arguments every time a new language is added, which is very time consuming. It would be great if we could simplify this and just check the top of the output with say a few call() lines.

Contribution

Happy to discuss implementation and review when a PR is up 😊

@andrewtavis andrewtavis added feature New feature or request help wanted Extra attention is needed good first issue Good for newcomers hacktoberfest Included as a part of Hacktoberfest labels Oct 19, 2024
@OmarAI2003
Copy link
Contributor

I want to work on this real quick!

@andrewtavis andrewtavis moved this from Todo to In Progress in Scribe Board Oct 19, 2024
@andrewtavis
Copy link
Member Author

Thanks for the offer to help, @OmarAI2003!

@KesharwaniArpita
Copy link
Contributor

@OmarAI2003 Can I help too?

@OmarAI2003
Copy link
Contributor

Absolutely @KesharwaniArpita , would love Your help!
let's chat on Matrix, and I'll update you on what I've done so far!

@KesharwaniArpita
Copy link
Contributor

@andrewtavis and @OmarAI2003, Do you think we will need a JSON file containing the languages and their associated data types that dynamically changes once a new language is added(based on a python script)?
Then in the tests we will just need to call the language(key) and the associated data type(value)?

@OmarAI2003
Copy link
Contributor

OmarAI2003 commented Oct 19, 2024

Taking the list_languages() as an exmple, I propose this simple approach:

  1. Focus on Edge Cases: We could select a few representative languages with different properties in the JSON file. For instance:

    • English: A main language, represented at the root level of the JSON file.
    • Bokmål: A sub-language under Norwegian, which also has other sub-languages.
    • Nigerian: An isolated sub-language.
  2. Check Calls in Any Order: Instead of matching the exact order of calls, we can verify that the expected calls are present in the mock output. This will allow us to ensure the critical calls are made without requiring an exact sequence.

This approach could potentially be applied to other functions as well

I believe these changes would improve the efficiency and maintainability of our tests. Looking forward to your thoughts @KesharwaniArpita, @andrewtavis !

@OmarAI2003
Copy link
Contributor

@andrewtavis and @OmarAI2003, Do you think we will need a JSON file containing the languages and their associated data types that dynamically changes once a new language is added(based on a python script)? Then in the tests we will just need to call the language(key) and the associated data type(value)?

This iss an interesting idea!
I’m curious to learn more about how you see the dynamic JSON file working with our current test framework. Specifically, how do you see the Python script updating it when new languages are added?

@Khushalsarode
Copy link
Contributor

@KesharwaniArpita @OmarAI2003 can I join you guys?

@OmarAI2003
Copy link
Contributor

Yeah, of course! @Khushalsarode, sure! Why not? 😃😉😄

@Abhishek02bhardwaj
Copy link

Hi @andrewtavis and @OmarAI2003 I have been following the contribution period closely and this issue caught my eye but I didn't really want to interrupt in the contribution period so I wanted to stay put for a while. But I am really curious about this issue. I wanted to know that the arguments we are talking about in this issue, they are in the test_list_data_types_specific_language and test_list_data_types_all_languages which means that we have to update the hard coded expected calls every time a new data_type is added or every time a new language is added (currently the test_list_data_types_specific_language tests only for English. Do we want to extend it for other languages too?)

@KesharwaniArpita
Copy link
Contributor

KesharwaniArpita commented Oct 20, 2024

So narrowing down for this issue my idea is generalizing the test_list_data_types_specific_language function to dynamically handle all available languages. Instead of hardcoding "English" and its data types, the updated test will fetch languages dynamically (e.g., from language_metadata.json) and loop through each one. For each language, it will retrieve the associated data types, construct the expected mock_print calls, and assert them accordingly.

Also should we update the call structure or expected_calls by defining the necessary and optional datatype checks?

Do you think we will need a JSON file containing the languages and their associated data types that dynamically changes once a new language is added(based on a python script)?

This is more of a general idea I had while thinking about solving this issue which is still open for a discussion. I was thinking if there is any way we can associate a python script with the resource files so that they can be updated dynamically everytime a new language(we can read the folder names or so) is added and also if we can connect both the metadata files(just like list_data_types does, it connects each language to its datatype, but in this case we can do it for all the languages in the metadata file). What do you think about it? @andrewtavis @OmarAI2003 @Khushalsarode @Abhishek02bhardwaj

@KesharwaniArpita
Copy link
Contributor

Focus on Edge Cases: We could select a few representative languages with different properties in the JSON file. For instance:

English: A main language, represented at the root level of the JSON file.
Bokmål: A sub-language under Norwegian, which also has other sub-languages.
Nigerian: An isolated sub-language.

The cases make sense to me, we can actually divide the languages in these cases for testing. But there will be hardcoding required, am I right? @OmarAI2003

@Khushalsarode
Copy link
Contributor

Khushalsarode commented Oct 20, 2024

Focus on Edge Cases: We could select a few representative languages with different properties in the JSON file. For instance:
English: A main language, represented at the root level of the JSON file.
Bokmål: A sub-language under Norwegian, which also has other sub-languages.
Nigerian: An isolated sub-language.

The cases make sense to me, we can actually divide the languages in these cases for testing. But there will be hardcoding required, am I right? @OmarAI2003

May be we can look

  • create an json file containing language path with lost of data types path, then call the file and set parameter to be called for execution.
  • or other create json file for language only then if language then make path to specific language folder to execution test.
  • For dynamic using code we can accept the language parameters then then we will use os module to dynamically list and load what ever we desire.

@andrewtavis @KesharwaniArpita @OmarAI2003

@OmarAI2003
Copy link
Contributor

Thanks @KesharwaniArpita @Khushalsarode for all the great suggestions! I think these ideas are heading in an interesting direction, but they seem more relevant to dynamically checking the language directories or metadata files, which might not be directly aligned with the current issue's focus on simplifying the CLI list tests.

Also, this is more related to some ongoing work in the check directoryn around dynamically handling the language paths and metadata (see PRs #385 and #390). For this specific issue, I believe a focus on edge cases and verifying the presence of critical calls without relying on the exact order would address this efficiently.

What do you all think about narrowing down the scope to stay within the testing domain for this issue? We could explore the broader automation ideas maybe in the check dir as part of separate tasks.

@KesharwaniArpita
Copy link
Contributor

What do you all think about narrowing down the scope to stay within the testing domain for this issue? We could explore the broader automation ideas maybe in the check dir as part of separate tasks

Make sense

I believe a focus on edge cases and verifying the presence of critical calls without relying on the exact order would address this efficiently.

I also agree to this approach. Let's discuss this with Andrew as well and we can work out the details. Sounds good?

@OmarAI2003
Copy link
Contributor

Make sense

Fair enough!

I also agree to this approach. Let's discuss this with Andrew as well and we can work out the details. Sounds good?

I’m sure Andrew will share his thoughts once he’s had a chance to see this.

@andrewtavis
Copy link
Member Author

What I was getting at here was generally checking the headers and the first line of the output :) Big thing here is that we want people to be able to add a language as easily as possible. So we check that the header is formatted appropriately, and for each we test that the first entry as gotten from the the metadata files is correct. From there we can also check to make sure that the total of the call is the appropriate size. So for languages the first entry now is Arabic. Dynamically get the first value of the output and check that it's formatted correctly, and for the rest ask "Are there as many outputs as there should be?".

Let me know if this makes sense! Just trying to keep it robust but simple :)

@andrewtavis
Copy link
Member Author

Closed by #447 🚀 Thanks, @OmarAI2003!

@github-project-automation github-project-automation bot moved this from In Progress to Done in Scribe Board Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request good first issue Good for newcomers hacktoberfest Included as a part of Hacktoberfest help wanted Extra attention is needed
Projects
Archived in project
Development

No branches or pull requests

5 participants